From 32e7e97eafe6bb90414a987042f28a2090c80056 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 21 Jul 2026 16:18:57 -0700 Subject: [PATCH 1/9] feat(code): add Hooks v2 capability snapshots Freeze validated hook configuration into deterministic session snapshots backed by one event capability registry. --- .../deepagents_code/hooks/capabilities.py | 267 ++++++++++++++++++ libs/code/deepagents_code/hooks/loading.py | 241 ++++++++++++++++ libs/code/deepagents_code/hooks/migration.py | 90 ++++++ .../deepagents_code/hooks/models/config.py | 17 +- libs/code/deepagents_code/hooks/snapshot.py | 89 ++++-- .../unit_tests/hooks/models/test_models.py | 3 +- .../unit_tests/hooks/test_configuration.py | 228 +++++++++++++++ 7 files changed, 902 insertions(+), 33 deletions(-) create mode 100644 libs/code/deepagents_code/hooks/capabilities.py create mode 100644 libs/code/deepagents_code/hooks/loading.py create mode 100644 libs/code/deepagents_code/hooks/migration.py create mode 100644 libs/code/tests/unit_tests/hooks/test_configuration.py diff --git a/libs/code/deepagents_code/hooks/capabilities.py b/libs/code/deepagents_code/hooks/capabilities.py new file mode 100644 index 0000000000..446cf402e3 --- /dev/null +++ b/libs/code/deepagents_code/hooks/capabilities.py @@ -0,0 +1,267 @@ +"""Capability registry for Hooks v2 events.""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import StrEnum +from types import MappingProxyType +from typing import TYPE_CHECKING, Final, get_args + +from deepagents_code.hooks.models.domain import ( + HookEvent, + HookOwner, + NotificationDecision, + NotificationEvent, + PermissionRequestDecision, + PermissionRequestEvent, + PostToolUseDecision, + PostToolUseEvent, + PreToolUseDecision, + PreToolUseEvent, + SessionEndDecision, + SessionEndEvent, + SessionStartDecision, + SessionStartEvent, + StopDecision, + StopEvent, + SubagentStartDecision, + SubagentStartEvent, + SubagentStopDecision, + SubagentStopEvent, +) + +if TYPE_CHECKING: + from collections.abc import Mapping + + from pydantic import BaseModel + + +class HandlerType(StrEnum): + """Supported hook handler executor kinds.""" + + COMMAND = "command" + + +class PlainOutputPolicy(StrEnum): + """How non-JSON stdout is treated on a successful exit.""" + + IGNORE = "ignore" + CONTEXT = "context" + + +class ExitCodePolicy(StrEnum): + """How exit code 2 is interpreted for an event.""" + + DENY = "deny" + FEEDBACK = "feedback" + CONTINUE_LOOP = "continue_loop" + DIAGNOSE = "diagnose" + IGNORE = "ignore" + + +class AggregationPolicy(StrEnum): + """How matching handler effects are combined.""" + + CONTEXT = "context" + PERMISSION = "permission" + FEEDBACK_AND_CONTEXT = "feedback_and_context" + STOP_LOOP = "stop_loop" + SIDE_EFFECT = "side_effect" + + +DEFAULT_COMMAND_TIMEOUT_SECONDS = 600.0 +_SUPPORTED_MATCHER_FIELDS = frozenset( + {"cause", "tool_name", "notification_type", "agent_name"} +) + + +@dataclass(frozen=True, slots=True) +class HookEventSpec: + """Immutable capability description for one hook event.""" + + event: HookEvent + owner: HookOwner + event_model: type[BaseModel] + decision_model: type[BaseModel] + matcher_field: str | None + default_timeout_seconds: float + exit_code_policy: ExitCodePolicy + plain_output_policy: PlainOutputPolicy + aggregation_policy: AggregationPolicy + supported_handler_types: frozenset[HandlerType] + + +HOOK_EVENT_SPECS: Final[Mapping[HookEvent, HookEventSpec]] = MappingProxyType( + { + HookEvent.SESSION_START: HookEventSpec( + event=HookEvent.SESSION_START, + owner=HookOwner.CLIENT, + event_model=SessionStartEvent, + decision_model=SessionStartDecision, + matcher_field="cause", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DIAGNOSE, + plain_output_policy=PlainOutputPolicy.CONTEXT, + aggregation_policy=AggregationPolicy.CONTEXT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.SESSION_END: HookEventSpec( + event=HookEvent.SESSION_END, + owner=HookOwner.CLIENT, + event_model=SessionEndEvent, + decision_model=SessionEndDecision, + matcher_field="cause", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DIAGNOSE, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.SIDE_EFFECT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.PERMISSION_REQUEST: HookEventSpec( + event=HookEvent.PERMISSION_REQUEST, + owner=HookOwner.CLIENT, + event_model=PermissionRequestEvent, + decision_model=PermissionRequestDecision, + matcher_field="tool_name", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DENY, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.PERMISSION, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.NOTIFICATION: HookEventSpec( + event=HookEvent.NOTIFICATION, + owner=HookOwner.CLIENT, + event_model=NotificationEvent, + decision_model=NotificationDecision, + matcher_field="notification_type", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DIAGNOSE, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.SIDE_EFFECT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.PRE_TOOL_USE: HookEventSpec( + event=HookEvent.PRE_TOOL_USE, + owner=HookOwner.SERVER, + event_model=PreToolUseEvent, + decision_model=PreToolUseDecision, + matcher_field="tool_name", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DENY, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.PERMISSION, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.POST_TOOL_USE: HookEventSpec( + event=HookEvent.POST_TOOL_USE, + owner=HookOwner.SERVER, + event_model=PostToolUseEvent, + decision_model=PostToolUseDecision, + matcher_field="tool_name", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.FEEDBACK, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.FEEDBACK_AND_CONTEXT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.STOP: HookEventSpec( + event=HookEvent.STOP, + owner=HookOwner.SERVER, + event_model=StopEvent, + decision_model=StopDecision, + matcher_field=None, + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.CONTINUE_LOOP, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.STOP_LOOP, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.SUBAGENT_START: HookEventSpec( + event=HookEvent.SUBAGENT_START, + owner=HookOwner.SERVER, + event_model=SubagentStartEvent, + decision_model=SubagentStartDecision, + matcher_field="agent_name", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DIAGNOSE, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.CONTEXT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + HookEvent.SUBAGENT_STOP: HookEventSpec( + event=HookEvent.SUBAGENT_STOP, + owner=HookOwner.SERVER, + event_model=SubagentStopEvent, + decision_model=SubagentStopDecision, + matcher_field="agent_name", + default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, + exit_code_policy=ExitCodePolicy.DIAGNOSE, + plain_output_policy=PlainOutputPolicy.IGNORE, + aggregation_policy=AggregationPolicy.CONTEXT, + supported_handler_types=frozenset({HandlerType.COMMAND}), + ), + } +) + + +def assert_hook_event_registry_complete() -> None: + """Raise if any `HookEvent` is missing from the capability registry. + + Raises: + RuntimeError: If the registry is incomplete or internally inconsistent. + """ + expected = frozenset(HookEvent) + actual = frozenset(HOOK_EVENT_SPECS) + missing = expected - actual + extra = actual - expected + if missing: + msg = f"Missing HookEventSpec entries: {sorted(e.value for e in missing)}" + raise RuntimeError(msg) + if extra: + msg = f"Unexpected HookEventSpec entries: {sorted(e.value for e in extra)}" + raise RuntimeError(msg) + for event, spec in HOOK_EVENT_SPECS.items(): + if spec.event is not event: + msg = f"HookEventSpec key/value mismatch for {event.value}" + raise RuntimeError(msg) + if HandlerType.COMMAND not in spec.supported_handler_types: + msg = f"HookEventSpec for {event.value} must support command handlers" + raise RuntimeError(msg) + if ( + spec.matcher_field is not None + and spec.matcher_field not in _SUPPORTED_MATCHER_FIELDS + ): + msg = ( + f"HookEventSpec for {event.value} has unsupported matcher field " + f"{spec.matcher_field!r}" + ) + raise RuntimeError(msg) + _assert_model_discriminator(spec.event_model, event, "event") + _assert_model_discriminator(spec.decision_model, event, "decision") + + +def _assert_model_discriminator( + model: type[BaseModel], + event: HookEvent, + kind: str, +) -> None: + field = model.model_fields.get("event") + if field is None or get_args(field.annotation) != (event,): + msg = f"{kind.title()} model for {event.value} has an invalid discriminator" + raise RuntimeError(msg) + + +assert_hook_event_registry_complete() + + +def get_event_spec(event: HookEvent) -> HookEventSpec: + """Return the capability entry for `event`. + + Args: + event: Lifecycle event name. + + Returns: + The registered capability specification. + """ + return HOOK_EVENT_SPECS[event] diff --git a/libs/code/deepagents_code/hooks/loading.py b/libs/code/deepagents_code/hooks/loading.py new file mode 100644 index 0000000000..e260df65e1 --- /dev/null +++ b/libs/code/deepagents_code/hooks/loading.py @@ -0,0 +1,241 @@ +"""Validated Hooks v2 configuration loading, merging, and hashing. + +Precedence (highest first, earlier in reduction order): + +1. Project: `{cwd}/.deepagents/hooks.json` +2. User: `~/.deepagents/hooks.json` (or `config_dir/hooks.json` in tests) + +Sources are concatenated per event. Project groups precede user groups so a +project `continue: false` wins before lower-precedence handlers run. + +Legacy list-shaped documents are migrated only for events whose lifecycle +semantics genuinely match Hooks v2. `tool.use` is never treated as `PreToolUse`. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from pathlib import ( # noqa: TC003 - used in runtime dataclass fields and path ops + Path, +) + +from pydantic import ValidationError + +from deepagents_code.hooks.migration import ( + is_legacy_hooks_document, + migrate_legacy_hooks, +) +from deepagents_code.hooks.models.adapters import HOOKS_CONFIG_ADAPTER +from deepagents_code.hooks.models.config import HooksConfig, MatcherGroup +from deepagents_code.hooks.models.domain import HookDiagnostic, HookEvent +from deepagents_code.model_config import DEFAULT_CONFIG_DIR + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True, slots=True) +class LoadedHooksConfig: + """Validated configuration plus load diagnostics and source paths.""" + + config: HooksConfig + diagnostics: tuple[HookDiagnostic, ...] + sources: tuple[Path, ...] + snapshot_id: str + + +def project_hooks_path(cwd: Path) -> Path: + """Return the project-scoped hooks configuration path. + + Args: + cwd: Session working directory. + + Returns: + `{cwd}/.deepagents/hooks.json`. + """ + return cwd / ".deepagents" / "hooks.json" + + +def user_hooks_path(config_dir: Path | None = None) -> Path: + """Return the user-scoped hooks configuration path. + + Args: + config_dir: Alternate user config directory (tests). + + Returns: + `{config_dir}/hooks.json`, defaulting to `~/.deepagents/hooks.json`. + """ + return (config_dir or DEFAULT_CONFIG_DIR) / "hooks.json" + + +def load_hooks_config( + *, + cwd: Path, + config_dir: Path | None = None, + paths: Sequence[Path] | None = None, +) -> LoadedHooksConfig: + """Load, validate, merge, and hash Hooks v2 configuration. + + Args: + cwd: Session working directory used for project precedence. + config_dir: Alternate user config directory. + paths: Explicit source paths in precedence order (highest first). + When omitted, project then user paths are used. + + Returns: + Frozen load result with canonical `snapshot_id`. + """ + sources = ( + tuple(paths) + if paths is not None + else ( + project_hooks_path(cwd), + user_hooks_path(config_dir), + ) + ) + diagnostics: list[HookDiagnostic] = [] + merged: dict[HookEvent, list[MatcherGroup]] = {} + loaded_paths: list[Path] = [] + + for path in sources: + document, file_diagnostics = _read_hooks_document(path) + diagnostics.extend(file_diagnostics) + if document is None: + continue + loaded_paths.append(path) + for event, groups in document.hooks.items(): + merged.setdefault(event, []).extend(groups) + + config = HooksConfig(hooks=merged) + return LoadedHooksConfig( + config=config, + diagnostics=tuple(diagnostics), + sources=tuple(loaded_paths), + snapshot_id=compute_snapshot_id(config), + ) + + +def compute_snapshot_id(config: HooksConfig) -> str: + """Return the canonical SHA-256 snapshot id for `config`. + + Args: + config: Validated Hooks v2 configuration. + + Returns: + Lowercase hex digest of the canonical JSON serialization. + """ + return hashlib.sha256(canonical_hooks_bytes(config)).hexdigest() + + +def canonical_hooks_bytes(config: HooksConfig) -> bytes: + """Serialize configuration into a stable byte representation. + + Args: + config: Validated Hooks v2 configuration. + + Returns: + UTF-8 JSON with sorted keys, event order fixed to `HookEvent`, and + `None` fields omitted. Unsupported MVP fields such as `async` are + excluded so equivalent configs hash identically. + """ + payload = { + "hooks": { + event.value: [ + _canonical_group(group) for group in config.hooks.get(event, []) + ] + for event in HookEvent + if event in config.hooks + } + } + return json.dumps( + payload, + sort_keys=True, + separators=(",", ":"), + ensure_ascii=False, + ).encode("utf-8") + + +def _canonical_group(group: MatcherGroup) -> dict[str, object]: + raw = group.model_dump(mode="json", by_alias=True, exclude_none=True) + handlers: list[dict[str, object]] = [] + hooks_raw = raw.get("hooks") + if isinstance(hooks_raw, list): + for item in hooks_raw: + if not isinstance(item, dict): + continue + handler = {str(key): value for key, value in item.items() if key != "async"} + handlers.append(handler) + result: dict[str, object] = {"hooks": handlers} + matcher = raw.get("matcher") + if matcher is not None: + result["matcher"] = matcher + return result + + +def _read_hooks_document( + path: Path, +) -> tuple[HooksConfig | None, tuple[HookDiagnostic, ...]]: + if not path.is_file(): + return None, () + try: + raw = path.read_text(encoding="utf-8") + data = json.loads(raw) + except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: + message = f"Failed to read hooks config at {path}: {exc}" + logger.warning(message) + return None, ( + HookDiagnostic( + code="config_read_failed", + severity="warning", + message=message, + field=str(path), + ), + ) + + if is_legacy_hooks_document(data): + hooks = data.get("hooks", []) if isinstance(data, dict) else [] + if not isinstance(hooks, list): + return None, ( + HookDiagnostic( + code="invalid_config", + severity="warning", + message=f"Legacy hooks list missing at {path}", + field=str(path), + ), + ) + legacy_entries = [item for item in hooks if isinstance(item, Mapping)] + migrated = migrate_legacy_hooks(legacy_entries) + message = ( + f"Migrated semantically equivalent session.end hooks from {path}; " + "all other legacy events remain unmapped" + if migrated.hooks + else ( + f"Legacy hooks at {path} contained no events that are safe to " + "migrate to Hooks v2" + ) + ) + diagnostic = HookDiagnostic( + code="legacy_migrated" if migrated.hooks else "legacy_unmapped", + severity="debug", + message=message, + field=str(path), + ) + return migrated, (diagnostic,) + + try: + config = HOOKS_CONFIG_ADAPTER.validate_python(data) + except ValidationError as exc: + message = f"Invalid hooks config at {path}: {exc.title}" + logger.warning(message) + return None, ( + HookDiagnostic( + code="invalid_config", + severity="warning", + message=message, + field=str(path), + ), + ) + return config, () diff --git a/libs/code/deepagents_code/hooks/migration.py b/libs/code/deepagents_code/hooks/migration.py new file mode 100644 index 0000000000..7d98f4ab00 --- /dev/null +++ b/libs/code/deepagents_code/hooks/migration.py @@ -0,0 +1,90 @@ +"""Legacy dotted-event migration helpers for Hooks v2 configuration. + +These utilities are intentionally not activated at legacy dispatch call sites. +Lifecycle wiring belongs to later tickets; this module only converts +semantically equivalent config when an explicit loader asks for it. +""" + +from __future__ import annotations + +import shlex +from typing import TYPE_CHECKING + +from deepagents_code.hooks.models.config import ( + CommandHandlerSpec, + HooksConfig, + MatcherGroup, +) +from deepagents_code.hooks.models.domain import HookEvent + +if TYPE_CHECKING: + from collections.abc import Mapping, Sequence + +# Only legacy `session.end` has the same lifecycle boundary and side-effect-only +# behavior as its Hooks v2 counterpart. Legacy `session.start` runs once per +# prompt execution, `context.compact` runs before the offload operation, and +# `permission.request` is a batched observation that cannot return a decision. +_LEGACY_EVENT_MAP: dict[str, tuple[HookEvent, str | None]] = { + "session.end": (HookEvent.SESSION_END, None), +} + + +def migrate_legacy_hooks( + legacy_hooks: Sequence[Mapping[str, object]], +) -> HooksConfig: + """Convert legacy dotted-event hook entries into Hooks v2 configuration. + + Args: + legacy_hooks: Entries from the legacy `hooks.json` list form. + + Returns: + A validated Hooks v2 configuration containing only migratable events. + """ + grouped: dict[HookEvent, list[MatcherGroup]] = {} + for entry in legacy_hooks: + command = entry.get("command") + if not isinstance(command, list) or not command: + continue + if not all(isinstance(part, str) for part in command): + continue + argv = [part for part in command if isinstance(part, str)] + if len(argv) != len(command): + continue + events = entry.get("events") + event_names: list[str] + if events is None or events == []: + event_names = list(_LEGACY_EVENT_MAP) + elif isinstance(events, list): + event_names = [name for name in events if isinstance(name, str)] + else: + continue + shell_command = shlex.join(argv) + for event_name in event_names: + mapped = _LEGACY_EVENT_MAP.get(event_name) + if mapped is None: + continue + event, matcher = mapped + grouped.setdefault(event, []).append( + MatcherGroup( + matcher=matcher, + hooks=[CommandHandlerSpec(type="command", command=shell_command)], + ) + ) + return HooksConfig(hooks=grouped) + + +def is_legacy_hooks_document(data: object) -> bool: + """Return whether `data` looks like the legacy list-shaped hooks document. + + Args: + data: Parsed JSON root. + + Returns: + `True` when `hooks` is a list of command entries rather than an event map. + """ + if not isinstance(data, dict): + return False + hooks = data.get("hooks") + if not isinstance(hooks, list): + return False + return all(isinstance(item, dict) for item in hooks) diff --git a/libs/code/deepagents_code/hooks/models/config.py b/libs/code/deepagents_code/hooks/models/config.py index 8124d76da1..e24a13ae26 100644 --- a/libs/code/deepagents_code/hooks/models/config.py +++ b/libs/code/deepagents_code/hooks/models/config.py @@ -2,9 +2,9 @@ from __future__ import annotations -from typing import Literal, TypeAlias +from typing import Literal, Self, TypeAlias -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator from deepagents_code.hooks.models.domain import ( # ruff:ignore[typing-only-first-party-import] - Pydantic runtime annotation. HookEvent, @@ -13,6 +13,7 @@ class _ConfigModel(BaseModel): # Ignore unknown keys so newer external handler fields do not fail config load. + # Known-but-unsupported fields such as `async` are modeled explicitly and rejected. model_config = ConfigDict(extra="ignore") @@ -28,6 +29,18 @@ class CommandHandlerSpec(_ConfigModel): command: str timeout: float | None = None status_message: str | None = Field(default=None, alias="statusMessage") + async_: bool | None = Field(default=None, alias="async") + + @model_validator(mode="after") + def _reject_async_commands(self) -> Self: + if self.async_: + msg = "async command hooks are not supported in MVP" + raise ValueError(msg) + # Normalize explicit `async: false` to omitted so equivalent configs + # share a snapshot hash. + if self.async_ is False: + return self.model_copy(update={"async_": None}) + return self HandlerSpec: TypeAlias = CommandHandlerSpec diff --git a/libs/code/deepagents_code/hooks/snapshot.py b/libs/code/deepagents_code/hooks/snapshot.py index f4bb02b1ca..b3313af0db 100644 --- a/libs/code/deepagents_code/hooks/snapshot.py +++ b/libs/code/deepagents_code/hooks/snapshot.py @@ -8,6 +8,8 @@ from types import MappingProxyType from typing import TYPE_CHECKING +from deepagents_code.hooks.capabilities import get_event_spec +from deepagents_code.hooks.loading import compute_snapshot_id from deepagents_code.hooks.models.domain import ( HookDiagnostic, HookEvent, @@ -31,20 +33,6 @@ logger = logging.getLogger(__name__) -# Events whose matcher filters a field on the invocation. `Stop` has no matcher. -_MATCHABLE_EVENTS = frozenset( - { - HookEvent.SESSION_START, - HookEvent.SESSION_END, - HookEvent.PERMISSION_REQUEST, - HookEvent.NOTIFICATION, - HookEvent.PRE_TOOL_USE, - HookEvent.POST_TOOL_USE, - HookEvent.SUBAGENT_START, - HookEvent.SUBAGENT_STOP, - } -) - # Claude-compatible exact-match character set (letters, digits, _, -, spaces, ,, |). _EXACT_MATCHER = re.compile(r"^[\w\s,\-|]+$") @@ -75,10 +63,17 @@ class HooksSnapshot: """Immutable, declaration-ordered Hooks v2 runtime configuration.""" handlers: Mapping[HookEvent, tuple[HookHandler, ...]] + snapshot_id: str diagnostics: tuple[HookDiagnostic, ...] = () @classmethod - def from_config(cls, config: HooksConfig) -> HooksSnapshot: + def from_config( + cls, + config: HooksConfig, + *, + diagnostics: tuple[HookDiagnostic, ...] = (), + snapshot_id: str | None = None, + ) -> HooksSnapshot: """Build an immutable runtime snapshot from validated configuration. Invalid matcher groups are rejected at compile time with a warning @@ -86,22 +81,48 @@ def from_config(cls, config: HooksConfig) -> HooksSnapshot: Args: config: Validated Hooks v2 configuration. + diagnostics: Diagnostics retained from configuration loading. + snapshot_id: Optional precomputed canonical hash. When omitted, it + is derived from `config`. Returns: - A snapshot whose handler order and matchers cannot change in flight. + A snapshot whose handler order, matchers, and id cannot change. + + Raises: + ValueError: If `snapshot_id` disagrees with the canonical config. """ + canonical_id = compute_snapshot_id(config) + if snapshot_id is not None and snapshot_id != canonical_id: + msg = "Provided snapshot_id does not match canonical configuration" + raise ValueError(msg) expanded: dict[HookEvent, tuple[HookHandler, ...]] = {} - diagnostics: list[HookDiagnostic] = [] + compile_diagnostics: list[HookDiagnostic] = list(diagnostics) for event, groups in config.hooks.items(): + matcher_field = get_event_spec(event).matcher_field handlers: list[HookHandler] = [] for group_index, group in enumerate(groups): + if matcher_field is None and group.matcher not in {None, "", "*"}: + message = ( + f"Rejected hook group {event.value}:{group_index}: " + f"{event.value} does not support matchers" + ) + logger.warning(message) + compile_diagnostics.append( + HookDiagnostic( + code="unsupported_matcher", + severity="warning", + message=message, + field="matcher", + ) + ) + continue matcher, error = _compile_matcher(group.matcher) if error is not None: message = ( f"Rejected hook group {event.value}:{group_index}: {error}" ) logger.warning(message) - diagnostics.append( + compile_diagnostics.append( HookDiagnostic( code="invalid_matcher", severity="warning", @@ -125,7 +146,8 @@ def from_config(cls, config: HooksConfig) -> HooksSnapshot: expanded[event] = tuple(handlers) return cls( handlers=MappingProxyType(expanded), - diagnostics=tuple(diagnostics), + snapshot_id=canonical_id, + diagnostics=tuple(compile_diagnostics), ) def match(self, invocation: HookInvocation) -> HookMatch: @@ -139,11 +161,12 @@ def match(self, invocation: HookInvocation) -> HookMatch: snapshot itself, not per invocation. """ event = invocation.event.event - target = _match_target(invocation) + matcher_field = get_event_spec(event).matcher_field + target = _match_target(invocation, matcher_field) matched = tuple( handler for handler in self.handlers.get(event, ()) - if _handler_matches(handler, event, target) + if _handler_matches(handler, matcher_field, target) ) return HookMatch(handlers=matched) @@ -180,10 +203,10 @@ def _compile_matcher( def _handler_matches( handler: HookHandler, - event: HookEvent, + matcher_field: str | None, target: str | None, ) -> bool: - if event not in _MATCHABLE_EVENTS or handler.matcher is None: + if matcher_field is None or handler.matcher is None: return True if target is None: return False @@ -193,17 +216,23 @@ def _handler_matches( return matcher.search(target) is not None -def _match_target(invocation: HookInvocation) -> str | None: +def _match_target( + invocation: HookInvocation, + matcher_field: str | None, +) -> str | None: event = invocation.event - if isinstance( - event, - PermissionRequestEvent | PreToolUseEvent | PostToolUseEvent, + if matcher_field == "tool_name" and isinstance( + event, PermissionRequestEvent | PreToolUseEvent | PostToolUseEvent ): return to_wire_tool_name(event.call.name) - if isinstance(event, NotificationEvent): + if matcher_field == "notification_type" and isinstance(event, NotificationEvent): return event.notification.type - if isinstance(event, SessionStartEvent | SessionEndEvent): + if matcher_field == "cause" and isinstance( + event, SessionStartEvent | SessionEndEvent + ): return event.cause.value - if isinstance(event, SubagentStartEvent | SubagentStopEvent): + if matcher_field == "agent_name" and isinstance( + event, SubagentStartEvent | SubagentStopEvent + ): return event.agent.name return None diff --git a/libs/code/tests/unit_tests/hooks/models/test_models.py b/libs/code/tests/unit_tests/hooks/models/test_models.py index c252841cf2..e92e49b2f1 100644 --- a/libs/code/tests/unit_tests/hooks/models/test_models.py +++ b/libs/code/tests/unit_tests/hooks/models/test_models.py @@ -353,7 +353,7 @@ def test_hooks_config_ignores_unknown_handler_fields() -> None: { "type": "command", "command": "./check.sh", - "async": True, + "async": False, "futureHandlerField": "keep-parsing", } ], @@ -367,6 +367,7 @@ def test_hooks_config_ignores_unknown_handler_fields() -> None: handler = config.hooks[HookEvent.PRE_TOOL_USE][0].hooks[0] assert handler.command == "./check.sh" assert handler.timeout is None + assert handler.async_ is None def test_hooks_config_rejects_unsupported_handler_type() -> None: diff --git a/libs/code/tests/unit_tests/hooks/test_configuration.py b/libs/code/tests/unit_tests/hooks/test_configuration.py new file mode 100644 index 0000000000..8a7daa26f2 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_configuration.py @@ -0,0 +1,228 @@ +"""Unit tests for Hooks v2 configuration and snapshots.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest +from pydantic import ValidationError + +from deepagents_code.hooks.capabilities import ( + DEFAULT_COMMAND_TIMEOUT_SECONDS, + HOOK_EVENT_SPECS, + assert_hook_event_registry_complete, + get_event_spec, +) +from deepagents_code.hooks.loading import ( + canonical_hooks_bytes, + compute_snapshot_id, + load_hooks_config, +) +from deepagents_code.hooks.migration import migrate_legacy_hooks +from deepagents_code.hooks.models.config import HooksConfig +from deepagents_code.hooks.models.domain import HookEvent +from deepagents_code.hooks.snapshot import HooksSnapshot + +if TYPE_CHECKING: + from pathlib import Path + + +def test_registry_covers_all_hook_events() -> None: + assert_hook_event_registry_complete() + assert set(HOOK_EVENT_SPECS) == set(HookEvent) + assert ( + get_event_spec(HookEvent.SESSION_END).default_timeout_seconds + == DEFAULT_COMMAND_TIMEOUT_SECONDS + ) + assert get_event_spec(HookEvent.PERMISSION_REQUEST).matcher_field == "tool_name" + + +def test_load_hooks_config_precedence_and_snapshot_hash(tmp_path: Path) -> None: + user_dir = tmp_path / "user" + project_dir = tmp_path / "project" + user_dir.mkdir() + (project_dir / ".deepagents").mkdir(parents=True) + (user_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + {"hooks": [{"type": "command", "command": "user-hook"}]} + ] + } + } + ), + encoding="utf-8", + ) + (project_dir / ".deepagents" / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + {"hooks": [{"type": "command", "command": "project-hook"}]} + ] + } + } + ), + encoding="utf-8", + ) + + loaded = load_hooks_config(cwd=project_dir, config_dir=user_dir) + groups = loaded.config.hooks[HookEvent.SESSION_START] + + assert [group.hooks[0].command for group in groups] == [ + "project-hook", + "user-hook", + ] + assert loaded.snapshot_id == compute_snapshot_id(loaded.config) + assert loaded.snapshot_id == compute_snapshot_id( + HooksConfig.model_validate( + { + "hooks": { + "SessionStart": [ + {"hooks": [{"type": "command", "command": "project-hook"}]}, + {"hooks": [{"type": "command", "command": "user-hook"}]}, + ] + } + } + ) + ) + assert canonical_hooks_bytes(loaded.config).startswith(b'{"hooks":') + + +def test_legacy_migration_only_maps_exact_session_end_semantics( + tmp_path: Path, +) -> None: + migrated = migrate_legacy_hooks( + [ + {"command": ["echo", "start"], "events": ["session.start"]}, + {"command": ["echo", "compact"], "events": ["context.compact"]}, + {"command": ["echo", "tool"], "events": ["tool.use"]}, + {"command": ["echo", "end"], "events": ["session.end"]}, + {"command": ["echo", "perm"], "events": ["permission.request"]}, + ] + ) + + assert set(migrated.hooks) == {HookEvent.SESSION_END} + assert HookEvent.SESSION_START not in migrated.hooks + assert HookEvent.PRE_TOOL_USE not in migrated.hooks + + user_dir = tmp_path / "user" + user_dir.mkdir() + (user_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": [ + {"command": ["echo", "start"], "events": ["session.start"]}, + {"command": ["echo", "end"], "events": ["session.end"]}, + {"command": ["echo", "tool"], "events": ["tool.use"]}, + ] + } + ), + encoding="utf-8", + ) + loaded = load_hooks_config(cwd=tmp_path, config_dir=user_dir) + + assert HookEvent.SESSION_START not in loaded.config.hooks + assert HookEvent.SESSION_END in loaded.config.hooks + assert HookEvent.PRE_TOOL_USE not in loaded.config.hooks + assert any(item.code == "legacy_migrated" for item in loaded.diagnostics) + + +def test_invalid_config_is_diagnosed(tmp_path: Path) -> None: + config_dir = tmp_path / "user" + config_dir.mkdir() + path = config_dir / "hooks.json" + path.write_text( + '{"hooks":{"Stop":[{"hooks":[{"type":"http"}]}]}}', + encoding="utf-8", + ) + + loaded = load_hooks_config(cwd=tmp_path, config_dir=config_dir) + + assert loaded.config.hooks == {} + assert loaded.sources == () + assert [item.code for item in loaded.diagnostics] == ["invalid_config"] + assert loaded.diagnostics[0].field == str(path) + + +def test_async_command_config_is_rejected() -> None: + with pytest.raises(ValidationError, match="async"): + HooksConfig.model_validate( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "echo", + "async": True, + } + ] + } + ] + } + } + ) + + +def test_snapshot_id_is_immutable_and_stable() -> None: + config = HooksConfig.model_validate( + { + "hooks": { + "PreToolUse": [ + {"hooks": [{"type": "command", "command": "policy"}]}, + ] + } + } + ) + first = HooksSnapshot.from_config(config) + second = HooksSnapshot.from_config(config) + + assert first.snapshot_id == second.snapshot_id + assert len(first.snapshot_id) == 64 + + with_false_async = HooksConfig.model_validate( + { + "hooks": { + "PreToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": "policy", + "async": False, + } + ] + } + ] + } + } + ) + assert compute_snapshot_id(with_false_async) == first.snapshot_id + assert with_false_async.hooks[HookEvent.PRE_TOOL_USE][0].hooks[0].async_ is None + + with pytest.raises(ValueError, match="canonical"): + HooksSnapshot.from_config(config, snapshot_id="not-the-canonical-id") + + +def test_snapshot_rejects_matcher_for_unmatchable_event() -> None: + snapshot = HooksSnapshot.from_config( + HooksConfig.model_validate( + { + "hooks": { + "Stop": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "stop"}], + } + ] + } + } + ) + ) + + assert snapshot.handlers[HookEvent.STOP] == () + assert [item.code for item in snapshot.diagnostics] == ["unsupported_matcher"] From f034bcd86ea6cd44682e148d965d2656c58b448c Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 21 Jul 2026 16:18:57 -0700 Subject: [PATCH 2/9] fix(code): harden Hooks v2 command execution Apply event-aware output policies and isolate command handlers with sanitized environments, bounded terminal output, and process-group cleanup. --- libs/code/deepagents_code/hooks/engine.py | 39 +- libs/code/deepagents_code/hooks/env.py | 55 +++ .../deepagents_code/hooks/models/domain.py | 1 + libs/code/deepagents_code/hooks/projection.py | 128 ++++-- libs/code/deepagents_code/hooks/reducer.py | 240 +++++++++--- libs/code/deepagents_code/hooks/runner.py | 52 ++- libs/code/deepagents_code/hooks/snapshot.py | 2 +- libs/code/deepagents_code/hooks/terminal.py | 31 ++ libs/code/deepagents_code/hooks/tools.py | 73 ++-- .../unit_tests/hooks/models/test_models.py | 83 +++- .../tests/unit_tests/hooks/test_engine.py | 369 ++++++++++++++++-- .../tests/unit_tests/hooks/test_execution.py | 227 +++++++++++ 12 files changed, 1119 insertions(+), 181 deletions(-) create mode 100644 libs/code/deepagents_code/hooks/env.py create mode 100644 libs/code/deepagents_code/hooks/terminal.py create mode 100644 libs/code/tests/unit_tests/hooks/test_execution.py diff --git a/libs/code/deepagents_code/hooks/engine.py b/libs/code/deepagents_code/hooks/engine.py index 95831211e5..34b6f95630 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -6,16 +6,21 @@ from dataclasses import dataclass from typing import TYPE_CHECKING +from deepagents_code.hooks.capabilities import get_event_spec from deepagents_code.hooks.models.domain import HookDiagnostic -from deepagents_code.hooks.projection import serialize_hook_input +from deepagents_code.hooks.projection import ( + projection_diagnostics, + serialize_hook_input, +) from deepagents_code.hooks.reducer import reduce_hook_results from deepagents_code.hooks.runner import ( - DEFAULT_HOOK_TIMEOUT, MAX_HOOK_OUTPUT_BYTES, run_command_handler, ) if TYPE_CHECKING: + from pathlib import Path + from deepagents_code.hooks.models.domain import HookDecision, HookInvocation from deepagents_code.hooks.snapshot import HooksSnapshot @@ -25,10 +30,16 @@ class HookEngine: """Execute Hooks v2 invocations against one immutable snapshot.""" snapshot: HooksSnapshot - default_timeout: float = DEFAULT_HOOK_TIMEOUT + default_timeout: float | None = None max_output_bytes: int = MAX_HOOK_OUTPUT_BYTES - async def run(self, invocation: HookInvocation) -> HookDecision: + async def run( + self, + invocation: HookInvocation, + *, + transcript_path: Path | None = None, + agent_transcript_path: Path | None = None, + ) -> HookDecision: """Execute matching handlers and return a normalized decision. Matching handlers run concurrently with independent timeouts. Results @@ -37,13 +48,20 @@ async def run(self, invocation: HookInvocation) -> HookDecision: Args: invocation: Native lifecycle invocation. + transcript_path: Materialized client transcript path. + agent_transcript_path: Materialized subagent transcript path. Returns: The event-specific decision produced by ordered hook reduction. """ match = self.snapshot.match(invocation) + projected_diagnostics = projection_diagnostics(invocation) try: - payload = serialize_hook_input(invocation) + payload = serialize_hook_input( + invocation, + transcript_path=transcript_path, + agent_transcript_path=agent_transcript_path, + ) except (TypeError, ValueError) as exc: diagnostic = HookDiagnostic( code="projection_failed", @@ -56,17 +74,25 @@ async def run(self, invocation: HookInvocation) -> HookDecision: diagnostics=( *self.snapshot.diagnostics, *match.diagnostics, + *projected_diagnostics, diagnostic, ), ) + event = invocation.event.event + event_default = ( + self.default_timeout + if self.default_timeout is not None + else get_event_spec(event).default_timeout_seconds + ) results = await asyncio.gather( *( run_command_handler( handler, payload, cwd=invocation.context.cwd, - default_timeout=self.default_timeout, + event=event, + default_timeout=event_default, max_output_bytes=self.max_output_bytes, ) for handler in match.handlers @@ -78,5 +104,6 @@ async def run(self, invocation: HookInvocation) -> HookDecision: diagnostics=( *self.snapshot.diagnostics, *match.diagnostics, + *projected_diagnostics, ), ) diff --git a/libs/code/deepagents_code/hooks/env.py b/libs/code/deepagents_code/hooks/env.py new file mode 100644 index 0000000000..f5b9ee28f6 --- /dev/null +++ b/libs/code/deepagents_code/hooks/env.py @@ -0,0 +1,55 @@ +"""Sanitized subprocess environments for Hooks v2 command handlers.""" + +from __future__ import annotations + +import os +from typing import TYPE_CHECKING + +from deepagents_code.config_manifest import _SECRET_NAME_MARKERS + +if TYPE_CHECKING: + from collections.abc import Mapping + +_OTEL_PREFIX = "OTEL_" + + +def is_secret_env_name(name: str) -> bool: + """Return whether an environment variable name looks like secret material. + + Uses the same credential-name markers as the config manifest, compared + case-insensitively so mixed-case env names are handled consistently. + + Args: + name: Environment variable name. + + Returns: + `True` when the name matches the repository secret-name policy. + """ + upper = name.upper() + return any(marker in upper for marker in _SECRET_NAME_MARKERS) + + +def sanitize_hook_environ( + source: Mapping[str, str] | None = None, +) -> dict[str, str]: + """Build an inherited environment safe to pass to hook subprocesses. + + Removes OpenTelemetry exporter variables (matching the compatible harness) + and strips values whose names look like secrets. Hooks are user-authored + trusted code, but secret values should not be ambiently available. + + Args: + source: Environment to sanitize. Defaults to `os.environ`. + + Returns: + A new environment mapping suitable for `asyncio.create_subprocess_shell`. + """ + env = dict(os.environ if source is None else source) + sanitized: dict[str, str] = {} + for key, value in env.items(): + if key.startswith(_OTEL_PREFIX): + continue + if is_secret_env_name(key): + continue + sanitized[key] = value + return sanitized diff --git a/libs/code/deepagents_code/hooks/models/domain.py b/libs/code/deepagents_code/hooks/models/domain.py index 89eab5692d..198293fe8e 100644 --- a/libs/code/deepagents_code/hooks/models/domain.py +++ b/libs/code/deepagents_code/hooks/models/domain.py @@ -83,6 +83,7 @@ class ToolCallData(_DomainModel): id: str name: str args: JsonObject + mcp_server: str | None = None class AgentIdentity(_DomainModel): diff --git a/libs/code/deepagents_code/hooks/projection.py b/libs/code/deepagents_code/hooks/projection.py index 824ddf4c84..4ec4306ee7 100644 --- a/libs/code/deepagents_code/hooks/projection.py +++ b/libs/code/deepagents_code/hooks/projection.py @@ -9,6 +9,7 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.models.adapters import HOOK_WIRE_INPUT_ADAPTER from deepagents_code.hooks.models.domain import ( + HookDiagnostic, HookEvent, NotificationEvent, PermissionRequestEvent, @@ -49,53 +50,65 @@ from deepagents_code.hooks.models.wire import HookWireInput -class _CommonWireFields(TypedDict): +class _CoreWireFields(TypedDict): session_id: str transcript_path: str cwd: str - prompt_id: NotRequired[UUID | None] - permission_mode: WirePermissionMode - effort: NotRequired[Effort | None] + permission_mode: NotRequired[WirePermissionMode] + prompt_id: NotRequired[UUID] + effort: NotRequired[Effort] -def project_hook_input(invocation: HookInvocation) -> HookWireInput: +class _CommonWireFields(_CoreWireFields): + agent_id: NotRequired[str] + agent_type: NotRequired[str] + + +def project_hook_input( + invocation: HookInvocation, + *, + transcript_path: Path | None = None, + agent_transcript_path: Path | None = None, +) -> HookWireInput: """Project a native hook invocation into the compatible wire contract. Args: invocation: Native lifecycle invocation. + transcript_path: Materialized client transcript path. + agent_transcript_path: Materialized subagent transcript path. Returns: A validated event-specific wire input. Raises: TypeError: If the invocation carries an unsupported event model. + ValueError: If a required materialized transcript path is missing. """ - context = invocation.context event = invocation.event if isinstance(event, SessionStartEvent): result = SessionStartWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.SESSION_START, source=event.cause, model=event.model, ) elif isinstance(event, SessionEndEvent): result = SessionEndWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.SESSION_END, reason=event.cause, ) elif isinstance(event, PermissionRequestEvent): tool_name, tool_input = to_wire_call(event.call) result = PermissionRequestWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.PERMISSION_REQUEST, tool_name=tool_name, tool_input=tool_input, ) elif isinstance(event, NotificationEvent): result = NotificationWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.NOTIFICATION, message=event.notification.message, title=event.notification.title, @@ -104,7 +117,7 @@ def project_hook_input(invocation: HookInvocation) -> HookWireInput: elif isinstance(event, PreToolUseEvent): tool_name, tool_input = to_wire_call(event.call) result = PreToolUseWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.PRE_TOOL_USE, tool_name=tool_name, tool_input=tool_input, @@ -113,7 +126,7 @@ def project_hook_input(invocation: HookInvocation) -> HookWireInput: elif isinstance(event, PostToolUseEvent): tool_name, tool_input = to_wire_call(event.call) result = PostToolUseWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.POST_TOOL_USE, tool_name=tool_name, tool_input=tool_input, @@ -123,7 +136,7 @@ def project_hook_input(invocation: HookInvocation) -> HookWireInput: ) elif isinstance(event, StopEvent): result = StopWireInput( - **_common_fields(invocation), + **_common_fields(invocation, transcript_path), hook_event_name=HookEvent.STOP, stop_hook_active=event.continuation_count > 0, last_assistant_message=event.last_assistant_message, @@ -138,19 +151,22 @@ def project_hook_input(invocation: HookInvocation) -> HookWireInput: ) elif isinstance(event, SubagentStartEvent): result = SubagentStartWireInput( - **_common_fields(invocation), + **_core_fields(invocation, transcript_path), hook_event_name=HookEvent.SUBAGENT_START, agent_id=event.agent.id, agent_type=event.agent.name, ) elif isinstance(event, SubagentStopEvent): + if agent_transcript_path is None: + msg = "SubagentStop requires a materialized agent transcript path" + raise ValueError(msg) result = SubagentStopWireInput( - **_common_fields(invocation), + **_core_fields(invocation, transcript_path), hook_event_name=HookEvent.SUBAGENT_STOP, stop_hook_active=event.continuation_count > 0, agent_id=event.agent.id, agent_type=event.agent.name, - agent_transcript_path=str(_transcript_path(context.cwd, event.agent.id)), + agent_transcript_path=str(agent_transcript_path), last_assistant_message=event.last_assistant_message, background_tasks=[ BackgroundTaskWire.model_validate(task.model_dump()) @@ -174,38 +190,89 @@ def project_hook_input(invocation: HookInvocation) -> HookWireInput: return HOOK_WIRE_INPUT_ADAPTER.validate_python(payload) -def _common_fields(invocation: HookInvocation) -> _CommonWireFields: +def _core_fields( + invocation: HookInvocation, + transcript_path: Path | None, +) -> _CoreWireFields: context = invocation.context - return { + if transcript_path is None: + msg = "HookInvocation requires a materialized transcript path" + raise ValueError(msg) + fields: _CoreWireFields = { "session_id": context.thread_id, - "transcript_path": str(_transcript_path(context.cwd, context.thread_id)), + "transcript_path": str(transcript_path), "cwd": str(context.cwd), - "prompt_id": context.prompt_id, - "permission_mode": _permission_mode(context.approval_mode), - "effort": Effort(level=context.effort) if context.effort is not None else None, } + permission_mode = _permission_mode(context.approval_mode) + if permission_mode is not None: + fields["permission_mode"] = permission_mode + if context.prompt_id is not None: + fields["prompt_id"] = context.prompt_id + if context.effort is not None: + fields["effort"] = Effort(level=context.effort) + return fields + + +def _common_fields( + invocation: HookInvocation, + transcript_path: Path | None, +) -> _CommonWireFields: + fields: _CommonWireFields = {**_core_fields(invocation, transcript_path)} + agent = invocation.context.agent + if agent is not None: + fields["agent_id"] = agent.id + fields["agent_type"] = agent.name + return fields -def serialize_hook_input(invocation: HookInvocation) -> bytes: +def serialize_hook_input( + invocation: HookInvocation, + *, + transcript_path: Path | None = None, + agent_transcript_path: Path | None = None, +) -> bytes: """Serialize a hook invocation as validated compatible JSON. Args: invocation: Native lifecycle invocation. + transcript_path: Materialized client transcript path. + agent_transcript_path: Materialized subagent transcript path. Returns: Compact JSON bytes suitable for command stdin. """ return HOOK_WIRE_INPUT_ADAPTER.dump_json( - project_hook_input(invocation), + project_hook_input( + invocation, + transcript_path=transcript_path, + agent_transcript_path=agent_transcript_path, + ), by_alias=True, exclude_none=True, ) -def _permission_mode(mode: ApprovalMode) -> WirePermissionMode: +def projection_diagnostics(invocation: HookInvocation) -> tuple[HookDiagnostic, ...]: + """Return visible diagnostics for lossy domain-to-wire projection.""" + if invocation.context.approval_mode is ApprovalMode.AUTO: + return ( + HookDiagnostic( + code="unsupported_permission_mode", + severity="warning", + message=( + "AUTO approval mode has no proven compatible hook permission " + "mode and was omitted" + ), + field="permission_mode", + ), + ) + return () + + +def _permission_mode(mode: ApprovalMode) -> WirePermissionMode | None: return { ApprovalMode.MANUAL: WirePermissionMode.DEFAULT, - ApprovalMode.AUTO: WirePermissionMode.AUTO, + ApprovalMode.AUTO: None, ApprovalMode.YOLO: WirePermissionMode.BYPASS_PERMISSIONS, }[mode] @@ -213,12 +280,9 @@ def _permission_mode(mode: ApprovalMode) -> WirePermissionMode: def _notification_type(value: str) -> WireNotificationType: try: return WireNotificationType(value) - except ValueError: - return WireNotificationType.AGENT_NEEDS_INPUT - - -def _transcript_path(cwd: Path, identifier: str) -> Path: - return cwd / ".deepagents" / "transcripts" / f"{identifier}.jsonl" + except ValueError as exc: + msg = f"Unsupported notification type: {value}" + raise ValueError(msg) from exc def _tool_result(result: ToolMessage | Command[str]) -> JsonValue: diff --git a/libs/code/deepagents_code/hooks/reducer.py b/libs/code/deepagents_code/hooks/reducer.py index adb9b25c28..797ceab7c5 100644 --- a/libs/code/deepagents_code/hooks/reducer.py +++ b/libs/code/deepagents_code/hooks/reducer.py @@ -5,6 +5,7 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING +from deepagents_code.hooks.capabilities import ExitCodePolicy, get_event_spec from deepagents_code.hooks.models.domain import ( HookDecision, HookDiagnostic, @@ -32,6 +33,7 @@ SubagentStartSpecificOutput, SubagentStopSpecificOutput, ) +from deepagents_code.hooks.terminal import validate_terminal_sequence if TYPE_CHECKING: from collections.abc import Iterable @@ -41,15 +43,13 @@ from deepagents_code.hooks.runner import HandlerResult _PERMISSION_RANK = {"none": 0, "allow": 1, "ask": 2, "deny": 3} +MAX_STOP_CONTINUATIONS = 8 -# Exit 2 / decision:"block" cannot veto these lifecycle points in MVP. -_NON_BLOCKING_EVENTS = frozenset( - { - HookEvent.SESSION_START, - HookEvent.SESSION_END, - HookEvent.NOTIFICATION, - HookEvent.SUBAGENT_START, - } +_DEFERRED_SESSION_START_FIELDS = ( + ("initial_user_message", "initialUserMessage"), + ("session_title", "sessionTitle"), + ("watch_paths", "watchPaths"), + ("reload_skills", "reloadSkills"), ) @@ -87,10 +87,10 @@ def reduce_hook_results( state = _Reduction(diagnostics=list(diagnostics)) for result in results: state.diagnostics.extend(result.diagnostics) + if result.plain_output is not None: + state.context.append(result.plain_output) if result.output is not None: _merge_output(invocation, state, result.handler_id, result.output) - if not state.continue_processing: - break return _decision(invocation, state) @@ -100,13 +100,50 @@ def _merge_output( handler_id: str, output: HookWireOutput, ) -> None: - state.continue_processing = output.continue_ + state.continue_processing = state.continue_processing and output.continue_ if output.stop_reason is not None: - state.stop_reason = output.stop_reason + if output.continue_: + state.diagnostics.append( + HookDiagnostic( + code="ignored_stop_reason", + severity="warning", + message="stopReason is ignored while continue is true", + handler_id=handler_id, + field="stopReason", + ) + ) + elif state.stop_reason is None: + state.stop_reason = output.stop_reason + else: + state.diagnostics.append( + HookDiagnostic( + code="additional_stop_reason", + severity="warning", + message="A later stopReason was ignored; the first reason wins", + handler_id=handler_id, + field="stopReason", + ) + ) if output.system_message is not None and not output.suppress_output: state.user_notices.append(output.system_message) if output.terminal_sequence is not None: - state.terminal_sequences.append(output.terminal_sequence) + validated = validate_terminal_sequence(output.terminal_sequence) + if validated is None: + state.diagnostics.append( + HookDiagnostic( + code="invalid_terminal_sequence", + severity="warning", + message=( + "terminalSequence rejected; only OSC 0/1/2/9/99/777 " + "and BEL are allowed" + ), + handler_id=handler_id, + field="terminalSequence", + ) + ) + else: + state.terminal_sequences.append(validated) + _diagnose_extra_fields(state, handler_id, output) if output.decision == "block": _merge_block(invocation, state, handler_id, output.reason) @@ -127,6 +164,26 @@ def _merge_output( _merge_specific(invocation, state, handler_id, specific) +def _diagnose_extra_fields( + state: _Reduction, + handler_id: str, + output: HookWireOutput, +) -> None: + extras = getattr(output, "__pydantic_extra__", None) + if not extras: + return + for name in sorted(extras): + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message=f"Unsupported hook output field ignored: {name}", + handler_id=handler_id, + field=name, + ) + ) + + def _merge_block( invocation: HookInvocation, state: _Reduction, @@ -135,43 +192,73 @@ def _merge_block( ) -> None: message = reason or "Blocked by hook" event = invocation.event.event - if event in {HookEvent.PERMISSION_REQUEST, HookEvent.PRE_TOOL_USE}: + policy = get_event_spec(event).exit_code_policy + if policy is ExitCodePolicy.DENY: _merge_permission(state, PermissionEffect(behavior="deny", reason=message)) - elif event is HookEvent.STOP: - if ( - isinstance(invocation.event, StopEvent) - and invocation.event.continuation_count - ): - state.diagnostics.append(_loop_guard_diagnostic()) - else: - state.continue_loop = True - state.feedback.append(message) - elif event is HookEvent.POST_TOOL_USE: + return + if policy is ExitCodePolicy.FEEDBACK: state.feedback.append(message) - elif event is HookEvent.SUBAGENT_STOP: + return + if policy is ExitCodePolicy.CONTINUE_LOOP: + _apply_stop_continuation(invocation, state, message) + return + if policy is ExitCodePolicy.IGNORE: + return + # DIAGNOSE: exit 2 / decision:"block" is not a veto. SubagentStop still + # retains parent-visible context on the first attempt per the MVP matrix. + if event is HookEvent.SUBAGENT_STOP: if ( isinstance(invocation.event, SubagentStopEvent) and invocation.event.continuation_count ): state.diagnostics.append(_loop_guard_diagnostic()) - else: - state.context.append(message) - elif event in _NON_BLOCKING_EVENTS: - # Exit 2 / decision:"block" is not a veto for these events; keep going - # and surface the attempt so configs that expect Claude blocking see why - # dcode ignored it. + return state.diagnostics.append( HookDiagnostic( code="unsupported_block", severity="warning", - message=f"Block/exit 2 is not supported for {event.value}: {message}", + message=( + "Blocking SubagentStop is not supported in MVP; " + f"retained as parent context: {message}" + ), handler_id=handler_id, field="decision", ) ) - else: - state.stop_reason = message - state.continue_processing = False + state.context.append(message) + return + state.diagnostics.append( + HookDiagnostic( + code="unsupported_block", + severity="warning", + message=f"Block/exit 2 is not supported for {event.value}: {message}", + handler_id=handler_id, + field="decision", + ) + ) + + +def _apply_stop_continuation( + invocation: HookInvocation, + state: _Reduction, + message: str, +) -> None: + if not isinstance(invocation.event, StopEvent): + return + if invocation.event.continuation_count >= MAX_STOP_CONTINUATIONS: + state.diagnostics.append( + HookDiagnostic( + code="continuation_cap", + severity="warning", + message=( + f"Ignored Stop continuation after {MAX_STOP_CONTINUATIONS} " + "consecutive attempts" + ), + ) + ) + return + state.continue_loop = True + state.feedback.append(message) def _merge_specific( @@ -182,9 +269,33 @@ def _merge_specific( ) -> None: if isinstance(specific, SessionStartSpecificOutput): _append(state.context, specific.additional_context) + for attr, wire_name in _DEFERRED_SESSION_START_FIELDS: + value = getattr(specific, attr) + if value in (None, False, [], ""): + continue + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message=f"{wire_name} is not supported and was ignored", + handler_id=handler_id, + field=wire_name, + ) + ) elif isinstance(specific, PreToolUseSpecificOutput): _append(state.context, specific.additional_context) behavior = specific.permission_decision + if behavior == "defer": + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message="permissionDecision defer is not supported and was ignored", + handler_id=handler_id, + field="permissionDecision", + ) + ) + behavior = None if specific.updated_input is not None: _diagnose_unsupported_updated_input(state, handler_id) # Allow/ask coupled to mutation falls back to normal permission flow; @@ -192,23 +303,33 @@ def _merge_specific( if behavior in {"allow", "ask"}: behavior = None if behavior is not None: - normalized = "none" if behavior == "defer" else behavior _merge_permission( state, PermissionEffect( - behavior=normalized, + behavior=behavior, reason=specific.permission_decision_reason, ), ) elif isinstance(specific, PermissionRequestSpecificOutput): decision = specific.decision if decision.behavior == "allow": - if ( + has_updated_input = ( isinstance(decision, PermissionAllow) and decision.updated_input is not None - ): + ) + if has_updated_input: _diagnose_unsupported_updated_input(state, handler_id) - else: + if isinstance(decision, PermissionAllow) and decision.updated_permissions: + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message="updatedPermissions is not supported and was ignored", + handler_id=handler_id, + field="updatedPermissions", + ) + ) + if not has_updated_input: _merge_permission(state, PermissionEffect(behavior="allow")) else: _merge_permission( @@ -221,29 +342,36 @@ def _merge_specific( ) elif isinstance(specific, PostToolUseSpecificOutput): _append(state.context, specific.additional_context) + if specific.updated_tool_output is not None: + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message="updatedToolOutput is not supported and was ignored", + handler_id=handler_id, + field="updatedToolOutput", + ) + ) + if specific.updated_mcp_tool_output is not None: + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message="updatedMCPToolOutput is not supported and was ignored", + handler_id=handler_id, + field="updatedMCPToolOutput", + ) + ) elif isinstance(specific, StopSpecificOutput): if specific.additional_context is not None: - if ( - isinstance(invocation.event, StopEvent) - and invocation.event.continuation_count - ): - state.diagnostics.append(_loop_guard_diagnostic()) - else: - state.continue_loop = True - state.feedback.append(specific.additional_context) + _apply_stop_continuation(invocation, state, specific.additional_context) elif isinstance(specific, SubagentStartSpecificOutput): _append(state.context, specific.additional_context) elif ( isinstance(specific, SubagentStopSpecificOutput) and specific.additional_context is not None ): - if ( - isinstance(invocation.event, SubagentStopEvent) - and invocation.event.continuation_count - ): - state.diagnostics.append(_loop_guard_diagnostic()) - else: - state.context.append(specific.additional_context) + state.context.append(specific.additional_context) def _diagnose_unsupported_updated_input( @@ -264,7 +392,7 @@ def _diagnose_unsupported_updated_input( def _merge_permission(state: _Reduction, effect: PermissionEffect) -> None: - if _PERMISSION_RANK[effect.behavior] >= _PERMISSION_RANK[state.permission.behavior]: + if _PERMISSION_RANK[effect.behavior] > _PERMISSION_RANK[state.permission.behavior]: state.permission = effect diff --git a/libs/code/deepagents_code/hooks/runner.py b/libs/code/deepagents_code/hooks/runner.py index f4cb921dd5..f9a9479468 100644 --- a/libs/code/deepagents_code/hooks/runner.py +++ b/libs/code/deepagents_code/hooks/runner.py @@ -6,13 +6,21 @@ import contextlib import json import os +import signal +from contextlib import suppress from dataclasses import dataclass from typing import TYPE_CHECKING from pydantic import ValidationError +from deepagents_code.hooks.capabilities import ( + DEFAULT_COMMAND_TIMEOUT_SECONDS, + PlainOutputPolicy, + get_event_spec, +) +from deepagents_code.hooks.env import sanitize_hook_environ from deepagents_code.hooks.models.adapters import HOOK_WIRE_OUTPUT_ADAPTER -from deepagents_code.hooks.models.domain import HookDiagnostic +from deepagents_code.hooks.models.domain import HookDiagnostic, HookEvent from deepagents_code.hooks.models.wire import HookWireOutput if TYPE_CHECKING: @@ -21,10 +29,11 @@ from deepagents_code.hooks.snapshot import HookHandler -DEFAULT_HOOK_TIMEOUT = 10.0 +DEFAULT_HOOK_TIMEOUT = DEFAULT_COMMAND_TIMEOUT_SECONDS MAX_HOOK_OUTPUT_BYTES = 100_000 _READ_CHUNK_BYTES = 8192 _BLOCKING_EXIT_CODE = 2 +_TERMINATE_WAIT_TIMEOUT = 2.0 @dataclass(frozen=True, slots=True) @@ -34,6 +43,7 @@ class HandlerResult: handler_id: str output: HookWireOutput | None = None diagnostics: tuple[HookDiagnostic, ...] = () + plain_output: str | None = None async def run_command_handler( @@ -41,8 +51,10 @@ async def run_command_handler( payload: bytes, *, cwd: Path, + event: HookEvent | None = None, default_timeout: float = DEFAULT_HOOK_TIMEOUT, max_output_bytes: int = MAX_HOOK_OUTPUT_BYTES, + env: dict[str, str] | None = None, ) -> HandlerResult: """Run one hook command with bounded time and captured output. @@ -50,8 +62,11 @@ async def run_command_handler( handler: Snapshotted command handler. payload: Validated JSON sent to stdin. cwd: Working directory inherited from the invocation. + event: Event used for plain-output policy. Defaults to `handler.event`. default_timeout: Timeout used when the handler has no override. max_output_bytes: Maximum retained bytes for each output stream. + env: Optional environment override. Defaults to a sanitized copy of the + process environment. Returns: Validated protocol output and structured diagnostics. @@ -62,13 +77,14 @@ async def run_command_handler( if not handler.command.strip(): return _failure(handler.id, "invalid_command", "Hook command is empty") + resolved_event = event or handler.event try: # Shell form preserves pipes, redirects, globs, and $VAR expansion to # match the compatible command-hook contract (no separate args field). process = await asyncio.create_subprocess_shell( handler.command, cwd=cwd, - env=os.environ.copy(), + env=env if env is not None else sanitize_hook_environ(), stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, @@ -117,6 +133,9 @@ async def run_command_handler( if process.returncode == _BLOCKING_EXIT_CODE: reason = _decode(stderr).strip() or "Hook blocked the operation" + # Exit 2 ignores stdout JSON. Event-specific interpretation of the + # synthetic decision:"block" is owned by the capability registry via + # the reducer. return HandlerResult( handler_id=handler.id, output=HookWireOutput(decision="block", reason=reason), @@ -137,6 +156,14 @@ async def run_command_handler( try: decoded = json.loads(stdout) except (json.JSONDecodeError, UnicodeDecodeError): + plain = _decode(stdout).strip() + policy = get_event_spec(resolved_event).plain_output_policy + if policy is PlainOutputPolicy.CONTEXT and plain: + return HandlerResult( + handler_id=handler.id, + plain_output=plain, + diagnostics=tuple(diagnostics), + ) diagnostics.append( _diagnostic(handler.id, "malformed_json", "Hook output is not valid JSON") ) @@ -214,9 +241,22 @@ async def _read_bounded( async def _terminate(process: Process) -> None: - if process.returncode is None: - process.kill() - await process.wait() + """Kill the hook process group, then reap the direct child.""" + if process.returncode is not None: + return + if os.name == "posix" and process.pid is not None: + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + except OSError: + with suppress(OSError): + process.kill() + else: + with suppress(OSError): + process.kill() + with suppress(OSError, TimeoutError): + await asyncio.wait_for(process.wait(), timeout=_TERMINATE_WAIT_TIMEOUT) def _decode(value: bytes) -> str: diff --git a/libs/code/deepagents_code/hooks/snapshot.py b/libs/code/deepagents_code/hooks/snapshot.py index b3313af0db..408e5255c6 100644 --- a/libs/code/deepagents_code/hooks/snapshot.py +++ b/libs/code/deepagents_code/hooks/snapshot.py @@ -224,7 +224,7 @@ def _match_target( if matcher_field == "tool_name" and isinstance( event, PermissionRequestEvent | PreToolUseEvent | PostToolUseEvent ): - return to_wire_tool_name(event.call.name) + return to_wire_tool_name(event.call.name, mcp_server=event.call.mcp_server) if matcher_field == "notification_type" and isinstance(event, NotificationEvent): return event.notification.type if matcher_field == "cause" and isinstance( diff --git a/libs/code/deepagents_code/hooks/terminal.py b/libs/code/deepagents_code/hooks/terminal.py new file mode 100644 index 0000000000..e0ff1fa848 --- /dev/null +++ b/libs/code/deepagents_code/hooks/terminal.py @@ -0,0 +1,31 @@ +"""Terminal escape-sequence validation for hook output.""" + +from __future__ import annotations + +import re + +# OSC 0/1/2/9/99/777 terminated by BEL or ST, plus bare BEL. +_ALLOWED_SEQUENCE = re.compile( + r"(?:" + r"\x1b\](?:0|1|2|9|99|777);[^\x00-\x1f\x7f-\x9f]*(?:\x07|\x1b\\)" + r"|" + r"\x07" + r")+" +) + + +def validate_terminal_sequence(value: str) -> str | None: + """Return `value` when it is composed only of allowlisted sequences. + + Allowed tokens are OSC `0`/`1`/`2`/`9`/`99`/`777` (BEL or ST terminated) + and bare BEL. Any other escape or control content rejects the entire value. + + Args: + value: Candidate `terminalSequence` payload. + + Returns: + The original string when valid, otherwise `None`. + """ + if not value: + return None + return value if _ALLOWED_SEQUENCE.fullmatch(value) is not None else None diff --git a/libs/code/deepagents_code/hooks/tools.py b/libs/code/deepagents_code/hooks/tools.py index c7911fabc0..4e02ef1c0a 100644 --- a/libs/code/deepagents_code/hooks/tools.py +++ b/libs/code/deepagents_code/hooks/tools.py @@ -36,56 +36,48 @@ def _edit_input(args: JsonObject) -> JsonObject: return _select(args, "file_path", "old_string", "new_string", "replace_all") -def _read_input(args: JsonObject) -> JsonObject: - result = _select(args, "file_path", "limit") - if "offset" in args: - offset = args["offset"] - result["offset"] = ( - offset + 1 - if isinstance(offset, int) and not isinstance(offset, bool) - else offset - ) - return result - - -def _glob_input(args: JsonObject) -> JsonObject: - return _select(args, "pattern", "path") - - -def _grep_input(args: JsonObject) -> JsonObject: - result = _select(args, "path", "glob", "output_mode") - if "pattern" in args: - pattern = args["pattern"] - result["pattern"] = re.escape(pattern) if isinstance(pattern, str) else pattern - if "max_count" in args: - result["head_limit"] = args["max_count"] - return result - - -def _ls_input(args: JsonObject) -> JsonObject: - return _select(args, "path") - - _NATIVE_TO_WIRE: dict[str, tuple[str, Callable[[JsonObject], JsonObject]]] = { "execute": ("Bash", _bash_input), "write_file": ("Write", _write_input), "edit_file": ("Edit", _edit_input), - "read_file": ("Read", _read_input), - "glob": ("Glob", _glob_input), - "grep": ("Grep", _grep_input), - "ls": ("LS", _ls_input), } +_MCP_WIRE_RE = re.compile(r"^mcp__.+__.+$") + + +def format_mcp_wire_name(server: str, tool: str) -> str: + """Format a compatible MCP wire tool name. + + Args: + server: MCP server name. + tool: Bare MCP tool name. + + Returns: + `mcp__{server}__{tool}`. + """ + return f"mcp__{server}__{tool}" + -def to_wire_tool_name(name: str) -> str: +def to_wire_tool_name( + name: str, + *, + mcp_server: str | None = None, +) -> str: """Map a native tool name to the compatible wire tool name. Args: name: Native dcode or already-compatible tool name. + mcp_server: Owning MCP server when known from tool metadata. Returns: The Claude-compatible tool name used for matchers and wire payloads. """ + if _MCP_WIRE_RE.fullmatch(name) is not None: + return name + if mcp_server is not None: + prefix = f"{mcp_server}_" + tool = name.removeprefix(prefix) if name.startswith(prefix) else name + return format_mcp_wire_name(mcp_server, tool) adapter = _NATIVE_TO_WIRE.get(name) return adapter[0] if adapter is not None else name @@ -101,10 +93,12 @@ def to_wire_tool_input(name: str, args: JsonObject) -> JsonObject: JSON object suitable for `tool_input` on the wire. """ adapter = _NATIVE_TO_WIRE.get(name) - return adapter[1](args) if adapter is not None else args + return adapter[1](args) if adapter is not None else dict(args) -def to_wire_call(call: ToolCallData) -> tuple[str, JsonObject]: +def to_wire_call( + call: ToolCallData, +) -> tuple[str, JsonObject]: """Project a native tool call into compatible wire name and input. Args: @@ -113,4 +107,7 @@ def to_wire_call(call: ToolCallData) -> tuple[str, JsonObject]: Returns: `(tool_name, tool_input)` for matchers and wire projection. """ - return to_wire_tool_name(call.name), to_wire_tool_input(call.name, call.args) + name = to_wire_tool_name(call.name, mcp_server=call.mcp_server) + if call.mcp_server is not None or _MCP_WIRE_RE.fullmatch(call.name) is not None: + return name, dict(call.args) + return name, to_wire_tool_input(call.name, call.args) diff --git a/libs/code/tests/unit_tests/hooks/models/test_models.py b/libs/code/tests/unit_tests/hooks/models/test_models.py index e92e49b2f1..97e072a9eb 100644 --- a/libs/code/tests/unit_tests/hooks/models/test_models.py +++ b/libs/code/tests/unit_tests/hooks/models/test_models.py @@ -293,6 +293,15 @@ def test_domain_models_reject_unknown_fields() -> None: "unsupported": True, } ) + with pytest.raises(ValidationError): + HookContext.model_validate( + { + "thread_id": "thread-1", + "cwd": Path("/workspace"), + "approval_mode": ApprovalMode.MANUAL, + "transcript_path": "/tmp/transcript.jsonl", + } + ) def test_decision_union_selects_event_model() -> None: @@ -343,32 +352,70 @@ def test_hooks_config_validates_event_keys_and_aliases() -> None: ) -def test_hooks_config_ignores_unknown_handler_fields() -> None: - payload = { - "hooks": { - "PreToolUse": [ - { - "matcher": "Bash", - "hooks": [ +def test_hooks_config_rejects_async_and_ignores_unknown_fields() -> None: + with pytest.raises(ValidationError, match="async"): + HOOKS_CONFIG_ADAPTER.validate_python( + { + "hooks": { + "PreToolUse": [ { - "type": "command", - "command": "./check.sh", - "async": False, - "futureHandlerField": "keep-parsing", + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "./check.sh", + "async": True, + } + ], } - ], + ] } - ] - } - } - - config = HOOKS_CONFIG_ADAPTER.validate_python(payload) + } + ) + config = HOOKS_CONFIG_ADAPTER.validate_python( + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "./check.sh", + "futureHandlerField": "keep-parsing", + } + ], + } + ] + } + } + ) handler = config.hooks[HookEvent.PRE_TOOL_USE][0].hooks[0] assert handler.command == "./check.sh" assert handler.timeout is None assert handler.async_ is None + normalized = HOOKS_CONFIG_ADAPTER.validate_python( + { + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "./check.sh", + "async": False, + } + ], + } + ] + } + } + ) + assert normalized.hooks[HookEvent.PRE_TOOL_USE][0].hooks[0].async_ is None + def test_hooks_config_rejects_unsupported_handler_type() -> None: with pytest.raises(ValidationError): @@ -426,4 +473,6 @@ def test_transport_models_round_trip_typed_domain_payloads() -> None: assert HOOK_INVOCATION_REQUEST_ADAPTER.validate_json(request_json) == request assert HOOK_INVOCATION_RESPONSE_ADAPTER.validate_json(response_json) == response + assert b"transcript_path" not in request_json + assert b"agent_transcript_path" not in request_json assert UUID(str(response.invocation_id)) == invocation_id diff --git a/libs/code/tests/unit_tests/hooks/test_engine.py b/libs/code/tests/unit_tests/hooks/test_engine.py index 3cc6629531..176af399cf 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -69,8 +69,19 @@ def _context(tmp_path: Path, *, agent: AgentIdentity | None = None) -> HookConte ) +def _transcript_path(tmp_path: Path) -> Path: + return tmp_path / "thread.jsonl" + + +def _agent_transcript_path(tmp_path: Path) -> Path: + return tmp_path / "agent.jsonl" + + def _invocation(tmp_path: Path, event: HookDomainEvent) -> HookInvocation: - return HookInvocation(context=_context(tmp_path), event=event) + agent = getattr(event, "agent", None) + if not isinstance(agent, AgentIdentity): + agent = None + return HookInvocation(context=_context(tmp_path, agent=agent), event=event) def _config(hooks: dict[str, object]) -> HooksConfig: @@ -290,6 +301,25 @@ def test_snapshot_rejects_invalid_matcher_at_compile_time(tmp_path: Path) -> Non assert match.diagnostics == () +def test_snapshot_rejects_matcher_for_unmatchable_event() -> None: + snapshot = HooksSnapshot.from_config( + _config( + { + "Stop": [ + { + "matcher": "Bash", + "hooks": [{"type": "command", "command": "invalid"}], + }, + {"hooks": [{"type": "command", "command": "valid"}]}, + ] + } + ) + ) + + assert [item.command for item in snapshot.handlers[HookEvent.STOP]] == ["valid"] + assert [item.code for item in snapshot.diagnostics] == ["unsupported_matcher"] + + @pytest.mark.parametrize( ("event", "expected"), [ @@ -374,7 +404,15 @@ def test_projects_all_wire_events( invocation = _invocation(tmp_path, event) payload = HOOK_WIRE_INPUT_ADAPTER.dump_python( - project_hook_input(invocation), + project_hook_input( + invocation, + transcript_path=_transcript_path(tmp_path), + agent_transcript_path=( + _agent_transcript_path(tmp_path) + if isinstance(event, SubagentStopEvent) + else None + ), + ), mode="json", by_alias=True, exclude_none=True, @@ -384,7 +422,7 @@ def test_projects_all_wire_events( assert payload["session_id"] == "thread-1" assert payload["permission_mode"] == "default" assert payload["effort"] == {"level": "high"} - assert payload["transcript_path"].endswith("thread-1.jsonl") + assert payload["transcript_path"].endswith("thread.jsonl") def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: @@ -401,7 +439,10 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: ) payload = HOOK_WIRE_INPUT_ADAPTER.dump_python( - project_hook_input(invocation), + project_hook_input( + invocation, + transcript_path=_transcript_path(tmp_path), + ), mode="json", by_alias=True, exclude_none=True, @@ -410,6 +451,26 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: assert payload["tool_name"] == "Bash" assert payload["tool_input"] == {"command": "pwd"} + agent = AgentIdentity(id="agent-9", name="researcher") + nested = HookInvocation( + context=_context(tmp_path, agent=agent), + event=PreToolUseEvent( + event=HookEvent.PRE_TOOL_USE, + call=ToolCallData(id="call-2", name="ls", args={"path": "."}), + ), + ) + nested_payload = HOOK_WIRE_INPUT_ADAPTER.dump_python( + project_hook_input( + nested, + transcript_path=_transcript_path(tmp_path), + ), + mode="json", + by_alias=True, + exclude_none=True, + ) + assert nested_payload["agent_id"] == "agent-9" + assert nested_payload["agent_type"] == "researcher" + invocation = _invocation( tmp_path, PostToolUseEvent.model_construct( @@ -419,12 +480,88 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: ), ) - payload = json.loads(serialize_hook_input(invocation)) + payload = json.loads( + serialize_hook_input( + invocation, + transcript_path=_transcript_path(tmp_path), + ) + ) assert payload["tool_response"]["update"] == {"result": "done"} assert payload["tool_response"]["goto"] == [] +def test_projection_rejects_unknown_notification_and_omits_auto_mode( + tmp_path: Path, +) -> None: + unknown = HookInvocation( + context=HookContext( + thread_id="thread", + cwd=tmp_path, + approval_mode=ApprovalMode.MANUAL, + ), + event=NotificationEvent( + event=HookEvent.NOTIFICATION, + notification=DcodeNotification(type="invented", message="notice"), + ), + ) + with pytest.raises(ValueError, match="Unsupported notification type"): + project_hook_input( + unknown, + transcript_path=_transcript_path(tmp_path), + ) + + automatic = HookInvocation( + context=HookContext( + thread_id="thread", + cwd=tmp_path, + approval_mode=ApprovalMode.AUTO, + ), + event=SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + payload = HOOK_WIRE_INPUT_ADAPTER.dump_python( + project_hook_input( + automatic, + transcript_path=_transcript_path(tmp_path), + ), + mode="json", + by_alias=True, + exclude_none=True, + ) + assert "permission_mode" not in payload + + +async def test_engine_requires_client_materialized_transcript_path( + tmp_path: Path, +) -> None: + invocation = _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + snapshot = HooksSnapshot.from_config(HooksConfig(hooks={})) + + missing = await HookEngine(snapshot).run(invocation) + automatic = HookInvocation( + context=invocation.context.model_copy( + update={"approval_mode": ApprovalMode.AUTO} + ), + event=invocation.event, + ) + auto = await HookEngine(snapshot).run( + automatic, + transcript_path=_transcript_path(tmp_path), + ) + + assert [item.code for item in missing.diagnostics] == ["projection_failed"] + assert [item.code for item in auto.diagnostics] == ["unsupported_permission_mode"] + + @pytest.mark.parametrize( ("name", "args", "wire_name", "wire_input"), [ @@ -459,13 +596,13 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: ( "read_file", {"file_path": "/tmp/result.txt", "offset": 0, "limit": 100}, - "Read", - {"file_path": "/tmp/result.txt", "offset": 1, "limit": 100}, + "read_file", + {"file_path": "/tmp/result.txt", "offset": 0, "limit": 100}, ), ( "glob", {"pattern": "**/*.py", "path": "/tmp"}, - "Glob", + "glob", {"pattern": "**/*.py", "path": "/tmp"}, ), ( @@ -477,16 +614,16 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: "output_mode": "content", "max_count": 20, }, - "Grep", + "grep", { - "pattern": r"result\.\*", + "pattern": "result.*", "path": "/tmp", "glob": "*.txt", "output_mode": "content", - "head_limit": 20, + "max_count": 20, }, ), - ("ls", {"path": "/tmp"}, "LS", {"path": "/tmp"}), + ("ls", {"path": "/tmp"}, "ls", {"path": "/tmp"}), ("custom", {"value": 1}, "custom", {"value": 1}), ], ) @@ -514,7 +651,12 @@ def test_serializes_native_tool_message_as_json(tmp_path: Path) -> None: ), ) - payload = json.loads(serialize_hook_input(invocation)) + payload = json.loads( + serialize_hook_input( + invocation, + transcript_path=_transcript_path(tmp_path), + ) + ) assert payload["tool_response"]["content"] == [{"type": "text", "text": "done"}] @@ -546,7 +688,6 @@ async def test_runner_executes_shell_syntax(tmp_path: Path) -> None: ("code", "expected_code"), [ ("pass", None), - ("print('not json')", "malformed_json"), ("print('[]')", "invalid_output"), ("raise SystemExit(3)", "nonzero_exit"), ], @@ -566,6 +707,53 @@ async def test_runner_protocol_failures_are_structured( ) +async def test_runner_session_start_plain_stdout_is_context(tmp_path: Path) -> None: + code = "print('plain')" + handler = _handler(tmp_path, f"{sys.executable} -c {json.dumps(code)}") + + result = await run_command_handler( + handler, + b"{}", + cwd=tmp_path, + event=HookEvent.SESSION_START, + ) + + assert result.output is None + assert result.plain_output == "plain" + assert result.diagnostics == () + + +async def test_runner_pretool_plain_stdout_is_malformed(tmp_path: Path) -> None: + code = "print('not json')" + snapshot = HooksSnapshot.from_config( + _config( + { + "PreToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": f"{sys.executable} -c {json.dumps(code)}", + } + ] + } + ] + } + ) + ) + handler = snapshot.handlers[HookEvent.PRE_TOOL_USE][0] + + result = await run_command_handler( + handler, + b"{}", + cwd=tmp_path, + event=HookEvent.PRE_TOOL_USE, + ) + + assert result.output is None + assert [item.code for item in result.diagnostics] == ["malformed_json"] + + async def test_runner_turns_exit_two_stderr_into_block(tmp_path: Path) -> None: code = "import sys; print('protected', file=sys.stderr); raise SystemExit(2)" handler = _handler(tmp_path, f"{sys.executable} -c {json.dumps(code)}") @@ -606,8 +794,8 @@ async def test_runner_reports_launch_failure_and_bounded_streams( assert {item.code for item in bounded.diagnostics} == { "stdout_truncated", "stderr_truncated", - "malformed_json", } + assert bounded.plain_output == "x" * 10 def test_reducer_merges_session_context_and_common_fields(tmp_path: Path) -> None: @@ -623,7 +811,7 @@ def test_reducer_merges_session_context_and_common_fields(tmp_path: Path) -> Non output=HookWireOutput.model_validate( { "systemMessage": "notice", - "terminalSequence": "sequence", + "terminalSequence": "\x1b]9;done\x07", "hookSpecificOutput": { "hookEventName": "SessionStart", "additionalContext": "context one", @@ -651,7 +839,7 @@ def test_reducer_merges_session_context_and_common_fields(tmp_path: Path) -> Non assert decision.context == ["context one", "context two"] assert decision.user_notices == ["notice"] - assert decision.terminal_sequences == ["sequence"] + assert decision.terminal_sequences == ["\x1b]9;done\x07"] assert decision.continue_processing is False assert decision.stop_reason == "stop" @@ -814,11 +1002,11 @@ def test_reducer_covers_event_decision_shapes_and_loop_guards(tmp_path: Path) -> assert permission.permission.behavior == "deny" assert post_tool.feedback == ["feedback"] assert post_tool.context == ["context"] - assert stop.continue_loop is False + assert stop.continue_loop is True + assert stop.feedback == ["continue"] assert subagent_start.context == ["focus"] - assert subagent_stop.context == [] - assert stop.diagnostics[0].code == "continuation_guard" - assert subagent_stop.diagnostics[0].code == "continuation_guard" + assert subagent_stop.context == ["continue"] + assert subagent_stop.diagnostics == [] def test_reducer_guards_top_level_stop_blocks(tmp_path: Path) -> None: @@ -826,7 +1014,7 @@ def test_reducer_guards_top_level_stop_blocks(tmp_path: Path) -> None: tmp_path, StopEvent( event=HookEvent.STOP, - continuation_count=1, + continuation_count=8, last_assistant_message="Done", ), ) @@ -850,8 +1038,8 @@ def test_reducer_guards_top_level_stop_blocks(tmp_path: Path) -> None: assert isinstance(stop, StopDecision) assert isinstance(subagent, SubagentStopDecision) assert stop.continue_loop is False + assert stop.diagnostics[0].code == "continuation_cap" assert subagent.context == [] - assert stop.diagnostics[0].code == "continuation_guard" assert subagent.diagnostics[0].code == "continuation_guard" @@ -974,6 +1162,131 @@ def test_reducer_honors_deny_even_with_updated_input(tmp_path: Path) -> None: assert decision.diagnostics[0].code == "unsupported_field" +def test_reducer_keeps_stop_sticky_and_retains_siblings(tmp_path: Path) -> None: + invocation = _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + decision = reduce_hook_results( + invocation, + [ + HandlerResult( + handler_id="first", + output=HookWireOutput.model_validate( + {"continue": False, "stopReason": "first"} + ), + ), + HandlerResult( + handler_id="second", + output=HookWireOutput.model_validate( + { + "continue": False, + "stopReason": "second", + "hookSpecificOutput": { + "hookEventName": "SessionStart", + "additionalContext": "later context", + }, + } + ), + ), + HandlerResult( + handler_id="third", + diagnostics=( + HookDiagnostic( + code="sibling_failed", + severity="warning", + message="sibling diagnostic", + ), + ), + plain_output="plain sibling", + ), + ], + ) + + assert isinstance(decision, SessionStartDecision) + assert decision.continue_processing is False + assert decision.stop_reason == "first" + assert decision.context == ["later context", "plain sibling"] + assert {item.code for item in decision.diagnostics} == { + "additional_stop_reason", + "sibling_failed", + } + + +def test_reducer_same_rank_permission_is_first_wins(tmp_path: Path) -> None: + invocation = _invocation( + tmp_path, + PreToolUseEvent( + event=HookEvent.PRE_TOOL_USE, + call=ToolCallData(id="call", name="execute", args={}), + ), + ) + results = [ + HandlerResult( + handler_id=reason, + output=HookWireOutput.model_validate( + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "ask", + "permissionDecisionReason": reason, + } + } + ), + ) + for reason in ("first", "second") + ] + + decision = reduce_hook_results(invocation, results) + + assert isinstance(decision, PreToolUseDecision) + assert decision.permission.behavior == "ask" + assert decision.permission.reason == "first" + + +def test_permission_request_diagnoses_all_deferred_fields(tmp_path: Path) -> None: + invocation = _invocation( + tmp_path, + PermissionRequestEvent( + event=HookEvent.PERMISSION_REQUEST, + call=ToolCallData(id="call", name="execute", args={}), + ), + ) + output = HookWireOutput.model_validate( + { + "hookSpecificOutput": { + "hookEventName": "PermissionRequest", + "decision": { + "behavior": "allow", + "updatedInput": {"command": "changed"}, + "updatedPermissions": [ + { + "type": "setMode", + "mode": "default", + "destination": "session", + } + ], + }, + } + } + ) + + decision = reduce_hook_results( + invocation, + [HandlerResult(handler_id="deferred", output=output)], + ) + + assert isinstance(decision, PermissionRequestDecision) + assert decision.permission.behavior == "none" + assert {item.field for item in decision.diagnostics} == { + "updatedInput", + "updatedPermissions", + } + + async def test_engine_runs_handlers_concurrently(tmp_path: Path) -> None: first = tmp_path / "first.txt" second = tmp_path / "second.txt" @@ -1019,7 +1332,10 @@ async def test_engine_runs_handlers_concurrently(tmp_path: Path) -> None: ), ) - decision = await HookEngine(snapshot).run(invocation) + decision = await HookEngine(snapshot).run( + invocation, + transcript_path=_transcript_path(tmp_path), + ) assert decision.continue_processing is False assert decision.stop_reason == "stop" @@ -1044,7 +1360,10 @@ async def test_engine_uses_captured_snapshot(tmp_path: Path) -> None: ), ) - decision = await HookEngine(snapshot).run(invocation) + decision = await HookEngine(snapshot).run( + invocation, + transcript_path=_transcript_path(tmp_path), + ) assert decision.diagnostics == [] diff --git a/libs/code/tests/unit_tests/hooks/test_execution.py b/libs/code/tests/unit_tests/hooks/test_execution.py new file mode 100644 index 0000000000..ac5a5eef55 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_execution.py @@ -0,0 +1,227 @@ +"""Unit tests for Hooks v2 execution safety and policies.""" + +from __future__ import annotations + +import asyncio +import os +from typing import TYPE_CHECKING + +import pytest + +from deepagents_code.approval_mode import ApprovalMode +from deepagents_code.hooks.capabilities import ( + ExitCodePolicy, + PlainOutputPolicy, + get_event_spec, +) +from deepagents_code.hooks.env import is_secret_env_name, sanitize_hook_environ +from deepagents_code.hooks.models.config import HooksConfig +from deepagents_code.hooks.models.domain import ( + HookContext, + HookEvent, + HookInvocation, + SessionStartCause, + SessionStartDecision, + SessionStartEvent, + ToolCallData, +) +from deepagents_code.hooks.models.wire import HookWireOutput +from deepagents_code.hooks.reducer import MAX_STOP_CONTINUATIONS, reduce_hook_results +from deepagents_code.hooks.runner import HandlerResult, run_command_handler +from deepagents_code.hooks.snapshot import HooksSnapshot +from deepagents_code.hooks.terminal import validate_terminal_sequence +from deepagents_code.hooks.tools import format_mcp_wire_name, to_wire_call + +if TYPE_CHECKING: + from pathlib import Path + + from deepagents_code.hooks.snapshot import HookHandler + + +def _invocation(tmp_path: Path) -> HookInvocation: + return HookInvocation( + context=HookContext( + thread_id="thread", + cwd=tmp_path, + approval_mode=ApprovalMode.MANUAL, + ), + event=SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + + +def _handler(command: str, *, timeout: float) -> HookHandler: + snapshot = HooksSnapshot.from_config( + HooksConfig.model_validate( + { + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": command, + "timeout": timeout, + } + ] + } + ] + } + } + ) + ) + return snapshot.handlers[HookEvent.SESSION_START][0] + + +def test_terminal_sequence_allowlist() -> None: + assert validate_terminal_sequence("\x1b]0;title\x07") == "\x1b]0;title\x07" + assert validate_terminal_sequence("\x1b]9;hello\x07") == "\x1b]9;hello\x07" + assert validate_terminal_sequence("\x07") == "\x07" + assert validate_terminal_sequence("\x1b]8;;https://example.com\x07") is None + assert validate_terminal_sequence("\x1b]9;line\nbreak\x07") is None + assert validate_terminal_sequence("\x1b[31mred\x1b[0m") is None + + +def test_reducer_rejects_invalid_terminal_and_deferred_fields( + tmp_path: Path, +) -> None: + decision = reduce_hook_results( + _invocation(tmp_path), + [ + HandlerResult( + handler_id="one", + output=HookWireOutput.model_validate( + { + "terminalSequence": "\x1b[31mnope", + "customField": "x", + "hookSpecificOutput": { + "hookEventName": "SessionStart", + "additionalContext": "ok", + "sessionTitle": "Nope", + "reloadSkills": True, + }, + } + ), + ) + ], + ) + + assert isinstance(decision, SessionStartDecision) + assert decision.terminal_sequences == [] + assert decision.context == ["ok"] + codes = {item.code for item in decision.diagnostics} + assert "invalid_terminal_sequence" in codes + assert "unsupported_field" in codes + + +def test_sanitized_env_strips_secrets_and_otel( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("SAFE_PATH", "/tmp") + monkeypatch.setenv("OPENAI_API_KEY", "placeholder") + monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost") + monkeypatch.setenv("MY_TOKEN", "placeholder") + monkeypatch.setenv("PYTHONPATH", "/opt/lib") + monkeypatch.setenv("HOME", "/home/user") + + env = sanitize_hook_environ() + + assert env["SAFE_PATH"] == "/tmp" + assert env["PYTHONPATH"] == "/opt/lib" + assert env["HOME"] == "/home/user" + assert "OPENAI_API_KEY" not in env + assert "OTEL_EXPORTER_OTLP_ENDPOINT" not in env + assert "MY_TOKEN" not in env + assert is_secret_env_name("ANTHROPIC_API_KEY") + assert is_secret_env_name("openai_api_key") + + +def test_mcp_tool_mapping_requires_resolved_metadata() -> None: + assert format_mcp_wire_name("github", "create_issue") == "mcp__github__create_issue" + call = ToolCallData( + id="1", + name="create_issue", + args={"title": "x"}, + mcp_server="github", + ) + + assert to_wire_call(call) == ("mcp__github__create_issue", {"title": "x"}) + assert to_wire_call(ToolCallData(id="2", name="github_create_issue", args={})) == ( + "github_create_issue", + {}, + ) + + +def test_exit_and_plain_output_policies_match_registry() -> None: + assert ( + get_event_spec(HookEvent.PRE_TOOL_USE).exit_code_policy is ExitCodePolicy.DENY + ) + assert ( + get_event_spec(HookEvent.POST_TOOL_USE).exit_code_policy + is ExitCodePolicy.FEEDBACK + ) + assert ( + get_event_spec(HookEvent.STOP).exit_code_policy is ExitCodePolicy.CONTINUE_LOOP + ) + assert ( + get_event_spec(HookEvent.SESSION_START).plain_output_policy + is PlainOutputPolicy.CONTEXT + ) + assert MAX_STOP_CONTINUATIONS == 8 + + +@pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX-specific") +async def test_runner_kills_process_group_on_timeout(tmp_path: Path) -> None: + script = tmp_path / "hook.sh" + grandchild_pid = tmp_path / "grandchild.pid" + script.write_text( + f"#!/bin/sh\nsleep 30 &\necho $! > {grandchild_pid}\nwait\n", + encoding="utf-8", + ) + script.chmod(0o755) + + result = await run_command_handler( + _handler(str(script), timeout=0.05), + b"{}", + cwd=tmp_path, + default_timeout=0.05, + ) + + assert [item.code for item in result.diagnostics] == ["timeout"] + if grandchild_pid.is_file(): + pid = int(grandchild_pid.read_text(encoding="utf-8").strip()) + with pytest.raises(ProcessLookupError): + os.kill(pid, 0) + + +@pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX-specific") +async def test_runner_kills_process_group_on_cancellation(tmp_path: Path) -> None: + script = tmp_path / "cancel.sh" + grandchild_pid = tmp_path / "grandchild.pid" + script.write_text( + f"#!/bin/sh\nsleep 30 &\necho $! > {grandchild_pid}\nwait\n", + encoding="utf-8", + ) + script.chmod(0o755) + task = asyncio.create_task( + run_command_handler( + _handler(str(script), timeout=30), + b"{}", + cwd=tmp_path, + default_timeout=30, + ) + ) + for _ in range(50): + if grandchild_pid.is_file(): + break + await asyncio.sleep(0.01) + + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + if grandchild_pid.is_file(): + pid = int(grandchild_pid.read_text(encoding="utf-8").strip()) + with pytest.raises(ProcessLookupError): + os.kill(pid, 0) From e95b3855be19c1d61371183bbbbaa1235d6d8741 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 21 Jul 2026 16:18:57 -0700 Subject: [PATCH 3/9] feat(code): add Hooks v2 session transcripts Materialize private versioned transcript projections and expose them through a session-scoped client runtime. --- libs/code/deepagents_code/hooks/runtime.py | 156 ++++++ libs/code/deepagents_code/hooks/transcript.py | 463 ++++++++++++++++++ .../tests/unit_tests/hooks/test_transcript.py | 258 ++++++++++ 3 files changed, 877 insertions(+) create mode 100644 libs/code/deepagents_code/hooks/runtime.py create mode 100644 libs/code/deepagents_code/hooks/transcript.py create mode 100644 libs/code/tests/unit_tests/hooks/test_transcript.py diff --git a/libs/code/deepagents_code/hooks/runtime.py b/libs/code/deepagents_code/hooks/runtime.py new file mode 100644 index 0000000000..ec84bd0259 --- /dev/null +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -0,0 +1,156 @@ +"""Session-scoped client facade for the Hooks v2 runtime.""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import ( # noqa: TC003 - used in runtime fields and path joins + Path, +) +from typing import TYPE_CHECKING + +from deepagents_code.hooks.engine import HookEngine +from deepagents_code.hooks.loading import load_hooks_config +from deepagents_code.hooks.models.domain import ( + HookDecision, + HookInvocation, + SubagentStartEvent, + SubagentStopEvent, +) +from deepagents_code.hooks.snapshot import HooksSnapshot +from deepagents_code.hooks.transcript import TranscriptStore + +if TYPE_CHECKING: + from collections.abc import Sequence + + from langchain_core.messages import BaseMessage + + +@dataclass(frozen=True, slots=True) +class PreparedHookInvocation: + """Client-only materialization needed to build one hook wire envelope.""" + + invocation: HookInvocation + transcript_path: Path + transcript_revision: str + agent_transcript_path: Path | None = None + agent_transcript_revision: str | None = None + + +@dataclass(frozen=True, slots=True) +class HooksRuntime: + """Client-owned session runtime around an immutable Hooks snapshot. + + Owns configuration snapshot identity, transcript materialization, and the + `HookEngine`. Lifecycle call sites are intentionally not wired here. + """ + + snapshot: HooksSnapshot + transcripts: TranscriptStore + engine: HookEngine + cwd: Path + + @classmethod + def create( + cls, + *, + cwd: Path, + config_dir: Path | None = None, + transcript_root: Path | None = None, + ) -> HooksRuntime: + """Load configuration once and freeze a session runtime. + + Args: + cwd: Session working directory. + config_dir: Alternate user config directory for tests. + transcript_root: Alternate transcript store root. Defaults to + `{cwd}/.deepagents/transcripts`. + + Returns: + A runtime ready to execute invocations for this session. + """ + loaded = load_hooks_config(cwd=cwd, config_dir=config_dir) + snapshot = HooksSnapshot.from_config( + loaded.config, + diagnostics=loaded.diagnostics, + snapshot_id=loaded.snapshot_id, + ) + store = TranscriptStore( + transcript_root or (cwd / ".deepagents" / "transcripts") + ) + engine = HookEngine(snapshot) + return cls(snapshot=snapshot, transcripts=store, engine=engine, cwd=cwd) + + @property + def snapshot_id(self) -> str: + """Canonical configuration hash for this session.""" + return self.snapshot.snapshot_id + + def append_messages( + self, + thread_id: str, + messages: Sequence[BaseMessage], + *, + agent_id: str | None = None, + ) -> None: + """Buffer conversation messages into the client transcript store. + + Args: + thread_id: Conversation thread identifier. + messages: LangChain messages to project. + agent_id: Optional subagent scope. + """ + self.transcripts.append_messages(thread_id, messages, agent_id=agent_id) + + async def invoke(self, invocation: HookInvocation) -> HookDecision: + """Materialize transcripts, execute matching handlers, and return a decision. + + Args: + invocation: Domain lifecycle invocation. + + Returns: + Event-specific decision with notices, sequences, and diagnostics. + """ + prepared = self.prepare_invocation(invocation) + return await self.engine.run( + prepared.invocation, + transcript_path=prepared.transcript_path, + agent_transcript_path=prepared.agent_transcript_path, + ) + + def prepare_invocation( + self, + invocation: HookInvocation, + ) -> PreparedHookInvocation: + """Materialize client-only transcript paths and revision identity. + + Args: + invocation: Domain lifecycle invocation. + + Returns: + A prepared value kept outside domain and graph state. + """ + context = invocation.context + thread_handle = self.transcripts.materialize(context.thread_id) + agent_id: str | None = None + if isinstance(invocation.event, SubagentStartEvent | SubagentStopEvent): + agent_id = invocation.event.agent.id + elif context.agent is not None: + agent_id = context.agent.id + + agent_path: Path | None = None + agent_revision: str | None = None + if agent_id is not None: + agent_handle = self.transcripts.materialize( + context.thread_id, + agent_id=agent_id, + ) + agent_path = agent_handle.path + agent_revision = agent_handle.revision + + return PreparedHookInvocation( + invocation=invocation, + transcript_path=thread_handle.path, + transcript_revision=thread_handle.revision, + agent_transcript_path=agent_path, + agent_transcript_revision=agent_revision, + ) diff --git a/libs/code/deepagents_code/hooks/transcript.py b/libs/code/deepagents_code/hooks/transcript.py new file mode 100644 index 0000000000..ce46dfff99 --- /dev/null +++ b/libs/code/deepagents_code/hooks/transcript.py @@ -0,0 +1,463 @@ +"""Client-owned conversation transcript projections for Hooks v2. + +Materializes versioned per-thread and per-subagent JSONL files that hook +commands can read via `transcript_path` / `agent_transcript_path`. + +Lag semantics: + The on-disk JSONL may lag behind live checkpoint/UI state. Callers that need + the just-finished assistant turn must prefer `last_assistant_message` on + Stop/SubagentStop. `materialize()` flushes pending records immediately before + returning a path so hooks see a consistent snapshot of what the store has + accepted so far, not a live tail of the server checkpoint. +""" + +from __future__ import annotations + +import hashlib +import logging +import os +import re +import tempfile +import threading +import unicodedata +from contextlib import suppress +from dataclasses import dataclass, field +from pathlib import Path +from typing import TYPE_CHECKING, Literal +from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit + +from langchain_core.messages import ( + AIMessage, + BaseMessage, + HumanMessage, + SystemMessage, + ToolMessage, +) +from pydantic import BaseModel, ConfigDict + +from deepagents_code.hooks.env import is_secret_env_name +from deepagents_code.json_types import JSON_VALUE_ADAPTER, JsonValue + +if TYPE_CHECKING: + from collections.abc import Sequence + +logger = logging.getLogger(__name__) + +TRANSCRIPT_SCHEMA_VERSION = 1 +DEFAULT_RETENTION_REVISIONS = 20 +_FILE_MODE = 0o600 +_DIR_MODE = 0o700 +# Credential-style assignments. Bare names like PASSWORD= are matched via the +# trailing keyword alternatives when preceded by an underscore-separated prefix +# (for example OPENAI_API_KEY=), matching the repository secret-name policy. +_SECRET_ASSIGNMENT_RE = re.compile( + r"(?i)\b([A-Z][A-Z0-9_]*(?:API[_-]?KEY|KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)" + r"[A-Z0-9_]*)\s*=\s*([^\s,;]+)" +) +_BEARER_RE = re.compile(r"(?i)\b(Bearer)\s+[A-Za-z0-9._~+/=-]{8,}") +_PREFIXED_TOKEN_RE = re.compile( + r"(?\"']+", re.IGNORECASE) +_SAFE_PREFIX_RE = re.compile(r"[^a-z0-9]+") +_SAFE_PREFIX_LENGTH = 32 +_EMPTY_REVISION = hashlib.sha256(b"").hexdigest() + + +class TranscriptRecord(BaseModel): + """One JSONL record in a materialized transcript projection.""" + + model_config = ConfigDict(extra="forbid") + + schema_version: Literal[1] = TRANSCRIPT_SCHEMA_VERSION + sequence: int + record_id: str + timestamp: str | None = None + thread_id: str + agent_id: str | None = None + role: Literal["user", "assistant", "tool", "system"] + message_id: str | None = None + content: JsonValue + name: str | None = None + + +@dataclass(frozen=True, slots=True) +class TranscriptHandle: + """Identity of a materialized transcript file.""" + + path: Path + revision: str + thread_id: str + agent_id: str | None = None + + +@dataclass +class _TranscriptBuffer: + records: list[TranscriptRecord] = field(default_factory=list) + dirty: bool = False + revision: str = _EMPTY_REVISION + + +class TranscriptStore: + """Append-only JSONL transcript projections owned by the client process.""" + + def __init__( + self, + root: Path, + *, + retention_revisions: int = DEFAULT_RETENTION_REVISIONS, + ) -> None: + """Create a store rooted at `root`. + + Args: + root: Directory that will contain per-thread transcript files. + retention_revisions: Maximum prior `.bak-*` revisions retained per + transcript after each rewrite. + + Raises: + ValueError: If `retention_revisions` is negative. + """ + if retention_revisions < 0: + msg = "retention_revisions must be nonnegative" + raise ValueError(msg) + self.root = root.expanduser().resolve() + self.retention_revisions = retention_revisions + self._buffers: dict[tuple[str, str | None], _TranscriptBuffer] = {} + self._lock = threading.RLock() + _ensure_private_directories(self.root, self.root) + + def thread_path(self, thread_id: str) -> Path: + """Return the materialized path for a thread transcript. + + Args: + thread_id: Conversation thread identifier. + + Returns: + Absolute JSONL path for the thread. + """ + return self.root / f"{_safe_component(thread_id)}.jsonl" + + def agent_path(self, thread_id: str, agent_id: str) -> Path: + """Return the materialized path for a subagent transcript. + + Args: + thread_id: Parent conversation thread identifier. + agent_id: Subagent identifier. + + Returns: + Absolute JSONL path nested under the thread. + """ + return ( + self.root + / _safe_component(thread_id) + / "agents" + / f"{_safe_component(agent_id)}.jsonl" + ) + + def append_messages( + self, + thread_id: str, + messages: Sequence[BaseMessage], + *, + agent_id: str | None = None, + ) -> None: + """Append redacted message projections to the in-memory buffer. + + Args: + thread_id: Conversation thread identifier. + messages: LangChain messages to project. + agent_id: Optional subagent scope. + """ + with self._lock: + buffer = self._buffer(thread_id, agent_id) + for message in messages: + record = _record_from_message( + message, + thread_id=thread_id, + agent_id=agent_id, + sequence=len(buffer.records), + ) + if record is None: + continue + buffer.records.append(record) + buffer.dirty = True + + def materialize( + self, + thread_id: str, + *, + agent_id: str | None = None, + ) -> TranscriptHandle: + """Flush pending records and return the client-readable path. + + Args: + thread_id: Conversation thread identifier. + agent_id: Optional subagent scope. + + Returns: + Handle with path and content revision identity. + """ + with self._lock: + buffer = self._buffer(thread_id, agent_id) + path = ( + self.agent_path(thread_id, agent_id) + if agent_id is not None + else self.thread_path(thread_id) + ) + _ensure_private_directories(self.root, path.parent) + if path.is_file() and os.name != "nt": + path.chmod(_FILE_MODE) + if buffer.dirty or not path.is_file(): + revision = _write_transcript( + self.root, + path, + buffer.records, + self.retention_revisions, + ) + buffer.revision = revision + buffer.dirty = False + return TranscriptHandle( + path=path, + revision=buffer.revision, + thread_id=thread_id, + agent_id=agent_id, + ) + + def revision(self, thread_id: str, *, agent_id: str | None = None) -> str: + """Return the current revision id without forcing a flush. + + Args: + thread_id: Conversation thread identifier. + agent_id: Optional subagent scope. + + Returns: + Content revision string for the buffered projection. + """ + with self._lock: + buffer = self._buffer(thread_id, agent_id) + if buffer.dirty: + return _revision_for_records(buffer.records) + return buffer.revision + + def _buffer(self, thread_id: str, agent_id: str | None) -> _TranscriptBuffer: + key = (thread_id, agent_id) + buffer = self._buffers.get(key) + if buffer is None: + buffer = _TranscriptBuffer() + path = ( + self.agent_path(thread_id, agent_id) + if agent_id is not None + else self.thread_path(thread_id) + ) + if path.is_file(): + buffer.records, valid = _read_transcript(path) + buffer.revision = _revision_for_records(buffer.records) + buffer.dirty = not valid + self._buffers[key] = buffer + return buffer + + +def _record_from_message( + message: BaseMessage, + *, + thread_id: str, + agent_id: str | None, + sequence: int, +) -> TranscriptRecord | None: + if isinstance(message, HumanMessage): + role: Literal["user", "assistant", "tool", "system"] = "user" + elif isinstance(message, AIMessage): + role = "assistant" + elif isinstance(message, ToolMessage): + role = "tool" + elif isinstance(message, SystemMessage): + role = "system" + else: + return None + + raw = message.model_dump(mode="json") + content = JSON_VALUE_ADAPTER.validate_python( + redact_transcript_value(raw.get("content")) + ) + message_id = message.id if isinstance(message.id, str) else None + name = getattr(message, "name", None) + tool_name = name if isinstance(name, str) else None + record_id = message_id or f"{role}:{sequence}" + return TranscriptRecord( + sequence=sequence, + record_id=record_id, + thread_id=thread_id, + agent_id=agent_id, + role=role, + message_id=message_id, + content=content, + name=tool_name, + ) + + +def redact_transcript_value(value: object) -> JsonValue: + """Redact secret-like strings inside transcript content. + + Args: + value: Arbitrary message content. + + Returns: + JSON-compatible content with URLs/credentials scrubbed. + """ + if isinstance(value, str): + return _redact_text(value) + if isinstance(value, list): + return [redact_transcript_value(item) for item in value] + if isinstance(value, dict): + return { + str(key): ( + "[redacted]" + if is_secret_env_name(str(key)) + else redact_transcript_value(item) + ) + for key, item in value.items() + } + return JSON_VALUE_ADAPTER.validate_python(value) + + +def _redact_text(text: str) -> str: + redacted = _SECRET_ASSIGNMENT_RE.sub( + lambda match: f"{match.group(1)}=[redacted]", + text, + ) + redacted = _BEARER_RE.sub(lambda match: f"{match.group(1)} [redacted]", redacted) + redacted = _PREFIXED_TOKEN_RE.sub("[redacted]", redacted) + redacted = _JWT_RE.sub("[redacted]", redacted) + return _URL_RE.sub(lambda match: _redact_url(match.group(0)), redacted) + + +def _redact_url(value: str) -> str: + try: + parsed = urlsplit(value) + hostname = parsed.hostname or "" + port = parsed.port + except ValueError: + return "[redacted URL]" + if ":" in hostname and not hostname.startswith("["): + hostname = f"[{hostname}]" + netloc = f"{hostname}:{port}" if port is not None else hostname + query_items = parse_qsl(parsed.query, keep_blank_values=True) + query = urlencode([(key, "[redacted]") for key, _value in query_items]) + fragment = "[redacted]" if parsed.fragment else "" + return urlunsplit((parsed.scheme, netloc, parsed.path, query, fragment)) + + +def _write_transcript( + root: Path, + path: Path, + records: Sequence[TranscriptRecord], + retention_revisions: int, +) -> str: + _ensure_private_directories(root, path.parent) + payload = "".join( + record.model_dump_json(exclude_none=True) + "\n" for record in records + ) + revision = hashlib.sha256(payload.encode("utf-8")).hexdigest() + fd, raw_tmp = tempfile.mkstemp(dir=path.parent, suffix=".tmp") + tmp_path = Path(raw_tmp) + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + if os.name != "nt": + tmp_path.chmod(_FILE_MODE) + # Copy the previous revision aside first, then atomically replace the + # live path so concurrent readers never observe a missing file. + if path.exists(): + prior_payload = path.read_bytes() + prior_revision = hashlib.sha256(prior_payload).hexdigest() + backup = path.with_suffix(path.suffix + f".bak-{prior_revision}") + _write_backup(backup, prior_payload) + _prune_backups(path, retention_revisions) + tmp_path.replace(path) + if os.name != "nt": + path.chmod(_FILE_MODE) + except OSError: + logger.warning("Failed to materialize transcript at %s", path, exc_info=True) + with suppress(OSError): + tmp_path.unlink(missing_ok=True) + raise + return revision + + +def _write_backup(path: Path, payload: bytes) -> None: + fd, raw_tmp = tempfile.mkstemp(dir=path.parent, suffix=".bak.tmp") + tmp_path = Path(raw_tmp) + try: + with os.fdopen(fd, "wb") as handle: + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + if os.name != "nt": + tmp_path.chmod(_FILE_MODE) + tmp_path.replace(path) + except OSError: + with suppress(OSError): + tmp_path.unlink(missing_ok=True) + raise + + +def _safe_component(identifier: str) -> str: + normalized = unicodedata.normalize("NFKD", identifier) + readable = normalized.encode("ascii", errors="ignore").decode("ascii").lower() + prefix = _SAFE_PREFIX_RE.sub("-", readable).strip("-")[:_SAFE_PREFIX_LENGTH] + digest = hashlib.sha256(identifier.encode("utf-8")).hexdigest() + return f"{prefix or 'id'}--{digest}" + + +def _ensure_private_directories(root: Path, target: Path) -> None: + root.mkdir(parents=True, exist_ok=True, mode=_DIR_MODE) + target.mkdir(parents=True, exist_ok=True, mode=_DIR_MODE) + if os.name == "nt": + return + root.chmod(_DIR_MODE) + relative = target.relative_to(root) + current = root + for part in relative.parts: + current /= part + current.chmod(_DIR_MODE) + + +def _prune_backups(path: Path, retention_revisions: int) -> None: + pattern = f"{path.name}.bak-*" + backups = sorted( + path.parent.glob(pattern), + key=lambda item: (item.stat().st_mtime_ns, item.name), + ) + excess = len(backups) - retention_revisions + for stale in backups[: max(0, excess)]: + with suppress(OSError): + stale.unlink() + + +def _read_transcript(path: Path) -> tuple[list[TranscriptRecord], bool]: + records: list[TranscriptRecord] = [] + try: + for line in path.read_text(encoding="utf-8").splitlines(): + if not line.strip(): + continue + records.append(TranscriptRecord.model_validate_json(line)) + except (OSError, ValueError): + logger.warning("Could not read transcript at %s", path, exc_info=True) + return [], False + return records, True + + +def _revision_for_records(records: Sequence[TranscriptRecord]) -> str: + payload = "".join( + record.model_dump_json(exclude_none=True) + "\n" for record in records + ) + return hashlib.sha256(payload.encode("utf-8")).hexdigest() diff --git a/libs/code/tests/unit_tests/hooks/test_transcript.py b/libs/code/tests/unit_tests/hooks/test_transcript.py new file mode 100644 index 0000000000..75136c5cbb --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_transcript.py @@ -0,0 +1,258 @@ +"""Unit tests for Hooks v2 transcripts and session runtime.""" + +from __future__ import annotations + +import json +import os +import stat +import sys +from concurrent.futures import ThreadPoolExecutor +from typing import TYPE_CHECKING + +import pytest +from langchain_core.messages import AIMessage, HumanMessage + +from deepagents_code.approval_mode import ApprovalMode +from deepagents_code.hooks.models.domain import ( + AgentIdentity, + HookContext, + HookEvent, + HookInvocation, + SessionStartCause, + SessionStartDecision, + SessionStartEvent, + SubagentStopEvent, +) +from deepagents_code.hooks.runtime import HooksRuntime +from deepagents_code.hooks.transcript import TranscriptStore, redact_transcript_value + +if TYPE_CHECKING: + from pathlib import Path + + +def test_transcript_store_permissions_atomicity_revision_redaction( + tmp_path: Path, +) -> None: + store = TranscriptStore(tmp_path / "transcripts", retention_revisions=2) + store.append_messages( + "thread-a", + [ + HumanMessage( + content=( + "token OPENAI_API_KEY=placeholder " + "https://example.com?access_token=opaque" + ) + ), + AIMessage(content="done"), + ], + ) + handle = store.materialize("thread-a") + + assert handle.path.is_file() + assert handle.path.is_absolute() + if os.name != "nt": + assert stat.S_IMODE(handle.path.stat().st_mode) == 0o600 + lines = handle.path.read_text(encoding="utf-8").splitlines() + assert len(lines) == 2 + first = json.loads(lines[0]) + assert "placeholder" not in first["content"] + assert "opaque" not in first["content"] + assert "[redacted]" in first["content"] + assert first["sequence"] == 0 + assert handle.revision == store.revision("thread-a") + assert "tool_calls" not in first + + previous = handle.path.read_text(encoding="utf-8") + store.append_messages("thread-a", [HumanMessage(content="again")]) + second = store.materialize("thread-a") + + assert second.revision != handle.revision + assert previous != second.path.read_text(encoding="utf-8") + backups = list(handle.path.parent.glob(f"{handle.path.name}.bak-*")) + assert backups + assert backups[0].read_text(encoding="utf-8") == previous + assert backups[0].name.endswith(handle.revision) + + agent = store.materialize("thread-a", agent_id="agent-1") + assert agent.path == store.agent_path("thread-a", "agent-1") + assert agent.path.is_absolute() + assert agent.path.is_file() + + redacted = redact_transcript_value({"token": "placeholder"}) + assert redacted == {"token": "[redacted]"} + + +def test_transcript_paths_are_safe_unique_and_private(tmp_path: Path) -> None: + root = tmp_path / "permissive" + root.mkdir(mode=0o777) + if os.name != "nt": + root.chmod(0o777) + store = TranscriptStore(root) + + identifiers = ["../escape", "a/b", "a\\b", "é", "e\u0301", "same"] + paths = [store.thread_path(identifier) for identifier in identifiers] + + assert len(set(paths)) == len(identifiers) + assert all(path.parent == store.root for path in paths) + assert all(".." not in path.name and "/" not in path.name for path in paths) + + agent = store.materialize("../escape", agent_id="../../agent") + assert agent.path.is_relative_to(store.root) + assert agent.path.is_file() + if os.name != "nt": + assert stat.S_IMODE(store.root.stat().st_mode) == 0o700 + assert stat.S_IMODE(agent.path.parent.parent.stat().st_mode) == 0o700 + assert stat.S_IMODE(agent.path.parent.stat().st_mode) == 0o700 + + with pytest.raises(ValueError, match="nonnegative"): + TranscriptStore(tmp_path / "invalid", retention_revisions=-1) + + +def test_transcript_redaction_covers_tokens_and_urls() -> None: + bare_token = "sk-" + ("x" * 24) + bearer = "Bearer " + ("y" * 24) + url = "https://user:password@example.com/path?access_token=opaque#fragment" + redacted = redact_transcript_value(f"{bare_token} {bearer} {url}") + + assert isinstance(redacted, str) + assert bare_token not in redacted + assert bearer not in redacted + assert "user:password" not in redacted + assert "opaque" not in redacted + assert "fragment" not in redacted + assert redacted.count("[redacted]") >= 2 + assert "%5Bredacted%5D" in redacted + + +def test_transcript_repairs_corrupt_existing_file_permissions(tmp_path: Path) -> None: + root = tmp_path / "transcripts" + initial = TranscriptStore(root) + path = initial.thread_path("thread") + path.write_text("{invalid json}\n", encoding="utf-8") + if os.name != "nt": + path.chmod(0o644) + + reloaded = TranscriptStore(root) + handle = reloaded.materialize("thread") + + assert handle.path.read_text(encoding="utf-8") == "" + assert handle.revision == reloaded.revision("thread") + if os.name != "nt": + assert stat.S_IMODE(handle.path.stat().st_mode) == 0o600 + + +def test_transcript_revision_is_deterministic_and_thread_safe(tmp_path: Path) -> None: + messages = [ + HumanMessage(id="user-1", content="first"), + AIMessage(id="assistant-1", content="second"), + ] + first = TranscriptStore(tmp_path / "first") + second = TranscriptStore(tmp_path / "second") + first.append_messages("thread", messages) + second.append_messages("thread", messages) + first_handle = first.materialize("thread") + second_handle = second.materialize("thread") + + assert first_handle.revision == second_handle.revision + assert first_handle.path.read_bytes() == second_handle.path.read_bytes() + + concurrent = TranscriptStore(tmp_path / "concurrent") + + def append(index: int) -> None: + concurrent.append_messages( + "thread", + [HumanMessage(id=f"message-{index}", content=str(index))], + ) + concurrent.materialize("thread") + + with ThreadPoolExecutor(max_workers=8) as executor: + list(executor.map(append, range(40))) + + handle = concurrent.materialize("thread") + records = [ + json.loads(line) + for line in handle.path.read_text(encoding="utf-8").splitlines() + ] + assert len(records) == 40 + assert [record["sequence"] for record in records] == list(range(40)) + assert len({record["message_id"] for record in records}) == 40 + assert handle.revision == concurrent.revision("thread") + + +async def test_runtime_materializes_paths_and_invokes(tmp_path: Path) -> None: + config_dir = tmp_path / "cfg" + config_dir.mkdir() + command = ( + "import json,sys; " + "payload=json.load(sys.stdin); " + "open(payload['transcript_path']).read(); " + "print(json.dumps({" + "'systemMessage':'ok'," + "'hookSpecificOutput':{" + "'hookEventName':'SessionStart'," + "'additionalContext':'from-hook'" + "}}))" + ) + (config_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": ( + f"{sys.executable} -c {json.dumps(command)}" + ), + } + ] + } + ] + } + } + ), + encoding="utf-8", + ) + runtime = HooksRuntime.create(cwd=tmp_path, config_dir=config_dir) + runtime.append_messages("thread-1", [HumanMessage(content="hi")]) + invocation = HookInvocation( + context=HookContext( + thread_id="thread-1", + cwd=tmp_path, + approval_mode=ApprovalMode.MANUAL, + ), + event=SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + + decision = await runtime.invoke(invocation) + prepared = runtime.prepare_invocation(invocation) + + assert isinstance(decision, SessionStartDecision) + assert decision.user_notices == ["ok"] + assert decision.context == ["from-hook"] + assert runtime.snapshot_id + assert prepared.transcript_path == runtime.transcripts.thread_path("thread-1") + assert prepared.transcript_path.is_file() + assert "transcript_path" not in invocation.context.model_fields_set + + agent = AgentIdentity(id="agent-1", name="researcher") + prepared_subagent = runtime.prepare_invocation( + HookInvocation( + context=invocation.context, + event=SubagentStopEvent( + event=HookEvent.SUBAGENT_STOP, + agent=agent, + continuation_count=0, + last_assistant_message="done", + ), + ) + ) + assert prepared_subagent.agent_transcript_path is not None + assert prepared_subagent.agent_transcript_path.is_file() + assert prepared_subagent.agent_transcript_path.is_relative_to( + runtime.transcripts.root + ) From 3fd18f1aafb10868a8c2297fb7f3545e843334d9 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 21 Jul 2026 16:52:21 -0700 Subject: [PATCH 4/9] fix(code): gate project hooks on workspace trust --- libs/code/deepagents_code/hooks/loading.py | 12 +++++--- .../unit_tests/hooks/test_configuration.py | 29 +++++++++++++++++-- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/libs/code/deepagents_code/hooks/loading.py b/libs/code/deepagents_code/hooks/loading.py index e260df65e1..a868a4fa67 100644 --- a/libs/code/deepagents_code/hooks/loading.py +++ b/libs/code/deepagents_code/hooks/loading.py @@ -74,6 +74,7 @@ def user_hooks_path(config_dir: Path | None = None) -> Path: def load_hooks_config( *, cwd: Path, + workspace_trusted: bool, config_dir: Path | None = None, paths: Sequence[Path] | None = None, ) -> LoadedHooksConfig: @@ -81,9 +82,11 @@ def load_hooks_config( Args: cwd: Session working directory used for project precedence. + workspace_trusted: Whether project-scoped hooks may be loaded. config_dir: Alternate user config directory. - paths: Explicit source paths in precedence order (highest first). - When omitted, project then user paths are used. + paths: Explicit trusted source paths in precedence order (highest first). + When omitted, project hooks are included only for trusted workspaces, + followed by user hooks. Returns: Frozen load result with canonical `snapshot_id`. @@ -92,8 +95,9 @@ def load_hooks_config( tuple(paths) if paths is not None else ( - project_hooks_path(cwd), - user_hooks_path(config_dir), + (project_hooks_path(cwd), user_hooks_path(config_dir)) + if workspace_trusted + else (user_hooks_path(config_dir),) ) ) diagnostics: list[HookDiagnostic] = [] diff --git a/libs/code/tests/unit_tests/hooks/test_configuration.py b/libs/code/tests/unit_tests/hooks/test_configuration.py index 8a7daa26f2..bc4c5787e9 100644 --- a/libs/code/tests/unit_tests/hooks/test_configuration.py +++ b/libs/code/tests/unit_tests/hooks/test_configuration.py @@ -68,7 +68,22 @@ def test_load_hooks_config_precedence_and_snapshot_hash(tmp_path: Path) -> None: encoding="utf-8", ) - loaded = load_hooks_config(cwd=project_dir, config_dir=user_dir) + untrusted = load_hooks_config( + cwd=project_dir, + workspace_trusted=False, + config_dir=user_dir, + ) + assert [ + group.hooks[0].command + for group in untrusted.config.hooks[HookEvent.SESSION_START] + ] == ["user-hook"] + assert untrusted.sources == (user_dir / "hooks.json",) + + loaded = load_hooks_config( + cwd=project_dir, + workspace_trusted=True, + config_dir=user_dir, + ) groups = loaded.config.hooks[HookEvent.SESSION_START] assert [group.hooks[0].command for group in groups] == [ @@ -122,7 +137,11 @@ def test_legacy_migration_only_maps_exact_session_end_semantics( ), encoding="utf-8", ) - loaded = load_hooks_config(cwd=tmp_path, config_dir=user_dir) + loaded = load_hooks_config( + cwd=tmp_path, + workspace_trusted=False, + config_dir=user_dir, + ) assert HookEvent.SESSION_START not in loaded.config.hooks assert HookEvent.SESSION_END in loaded.config.hooks @@ -139,7 +158,11 @@ def test_invalid_config_is_diagnosed(tmp_path: Path) -> None: encoding="utf-8", ) - loaded = load_hooks_config(cwd=tmp_path, config_dir=config_dir) + loaded = load_hooks_config( + cwd=tmp_path, + workspace_trusted=False, + config_dir=config_dir, + ) assert loaded.config.hooks == {} assert loaded.sources == () From 8d44e5aa274148431e587500f033a8bed303af2c Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 22 Jul 2026 12:14:43 -0700 Subject: [PATCH 5/9] cr --- .../deepagents_code/hooks/capabilities.py | 90 +++------ libs/code/deepagents_code/hooks/loading.py | 179 ++++++++++++++---- .../deepagents_code/hooks/models/config.py | 20 +- .../unit_tests/hooks/test_configuration.py | 93 ++++++++- 4 files changed, 269 insertions(+), 113 deletions(-) diff --git a/libs/code/deepagents_code/hooks/capabilities.py b/libs/code/deepagents_code/hooks/capabilities.py index 446cf402e3..93008986c3 100644 --- a/libs/code/deepagents_code/hooks/capabilities.py +++ b/libs/code/deepagents_code/hooks/capabilities.py @@ -5,7 +5,7 @@ from dataclasses import dataclass from enum import StrEnum from types import MappingProxyType -from typing import TYPE_CHECKING, Final, get_args +from typing import TYPE_CHECKING, Final, Literal, TypeAlias, assert_never from deepagents_code.hooks.models.domain import ( HookEvent, @@ -33,7 +33,7 @@ if TYPE_CHECKING: from collections.abc import Mapping - from pydantic import BaseModel + from deepagents_code.hooks.models.domain import BaseHookDecision, HookDomainEvent class HandlerType(StrEnum): @@ -70,9 +70,9 @@ class AggregationPolicy(StrEnum): DEFAULT_COMMAND_TIMEOUT_SECONDS = 600.0 -_SUPPORTED_MATCHER_FIELDS = frozenset( - {"cause", "tool_name", "notification_type", "agent_name"} -) +MatcherField: TypeAlias = Literal[ + "cause", "tool_name", "notification_type", "agent_name" +] @dataclass(frozen=True, slots=True) @@ -81,9 +81,9 @@ class HookEventSpec: event: HookEvent owner: HookOwner - event_model: type[BaseModel] - decision_model: type[BaseModel] - matcher_field: str | None + event_model: type[HookDomainEvent] + decision_model: type[BaseHookDecision] + matcher_field: MatcherField | None default_timeout_seconds: float exit_code_policy: ExitCodePolicy plain_output_policy: PlainOutputPolicy @@ -91,7 +91,7 @@ class HookEventSpec: supported_handler_types: frozenset[HandlerType] -HOOK_EVENT_SPECS: Final[Mapping[HookEvent, HookEventSpec]] = MappingProxyType( +_HOOK_EVENT_SPECS: Final[Mapping[HookEvent, HookEventSpec]] = MappingProxyType( { HookEvent.SESSION_START: HookEventSpec( event=HookEvent.SESSION_START, @@ -205,56 +205,6 @@ class HookEventSpec: ) -def assert_hook_event_registry_complete() -> None: - """Raise if any `HookEvent` is missing from the capability registry. - - Raises: - RuntimeError: If the registry is incomplete or internally inconsistent. - """ - expected = frozenset(HookEvent) - actual = frozenset(HOOK_EVENT_SPECS) - missing = expected - actual - extra = actual - expected - if missing: - msg = f"Missing HookEventSpec entries: {sorted(e.value for e in missing)}" - raise RuntimeError(msg) - if extra: - msg = f"Unexpected HookEventSpec entries: {sorted(e.value for e in extra)}" - raise RuntimeError(msg) - for event, spec in HOOK_EVENT_SPECS.items(): - if spec.event is not event: - msg = f"HookEventSpec key/value mismatch for {event.value}" - raise RuntimeError(msg) - if HandlerType.COMMAND not in spec.supported_handler_types: - msg = f"HookEventSpec for {event.value} must support command handlers" - raise RuntimeError(msg) - if ( - spec.matcher_field is not None - and spec.matcher_field not in _SUPPORTED_MATCHER_FIELDS - ): - msg = ( - f"HookEventSpec for {event.value} has unsupported matcher field " - f"{spec.matcher_field!r}" - ) - raise RuntimeError(msg) - _assert_model_discriminator(spec.event_model, event, "event") - _assert_model_discriminator(spec.decision_model, event, "decision") - - -def _assert_model_discriminator( - model: type[BaseModel], - event: HookEvent, - kind: str, -) -> None: - field = model.model_fields.get("event") - if field is None or get_args(field.annotation) != (event,): - msg = f"{kind.title()} model for {event.value} has an invalid discriminator" - raise RuntimeError(msg) - - -assert_hook_event_registry_complete() - - def get_event_spec(event: HookEvent) -> HookEventSpec: """Return the capability entry for `event`. @@ -264,4 +214,24 @@ def get_event_spec(event: HookEvent) -> HookEventSpec: Returns: The registered capability specification. """ - return HOOK_EVENT_SPECS[event] + match event: + case HookEvent.SESSION_START: + return _HOOK_EVENT_SPECS[HookEvent.SESSION_START] + case HookEvent.SESSION_END: + return _HOOK_EVENT_SPECS[HookEvent.SESSION_END] + case HookEvent.PERMISSION_REQUEST: + return _HOOK_EVENT_SPECS[HookEvent.PERMISSION_REQUEST] + case HookEvent.NOTIFICATION: + return _HOOK_EVENT_SPECS[HookEvent.NOTIFICATION] + case HookEvent.PRE_TOOL_USE: + return _HOOK_EVENT_SPECS[HookEvent.PRE_TOOL_USE] + case HookEvent.POST_TOOL_USE: + return _HOOK_EVENT_SPECS[HookEvent.POST_TOOL_USE] + case HookEvent.STOP: + return _HOOK_EVENT_SPECS[HookEvent.STOP] + case HookEvent.SUBAGENT_START: + return _HOOK_EVENT_SPECS[HookEvent.SUBAGENT_START] + case HookEvent.SUBAGENT_STOP: + return _HOOK_EVENT_SPECS[HookEvent.SUBAGENT_STOP] + case _: + assert_never(event) diff --git a/libs/code/deepagents_code/hooks/loading.py b/libs/code/deepagents_code/hooks/loading.py index a868a4fa67..e5af0d2e60 100644 --- a/libs/code/deepagents_code/hooks/loading.py +++ b/libs/code/deepagents_code/hooks/loading.py @@ -2,14 +2,14 @@ Precedence (highest first, earlier in reduction order): -1. Project: `{cwd}/.deepagents/hooks.json` +1. Project: `{project_root}/.deepagents/hooks.json` 2. User: `~/.deepagents/hooks.json` (or `config_dir/hooks.json` in tests) Sources are concatenated per event. Project groups precede user groups so a project `continue: false` wins before lower-precedence handlers run. Legacy list-shaped documents are migrated only for events whose lifecycle -semantics genuinely match Hooks v2. `tool.use` is never treated as `PreToolUse`. +semantics genuinely match Hooks v2. """ from __future__ import annotations @@ -29,12 +29,16 @@ is_legacy_hooks_document, migrate_legacy_hooks, ) -from deepagents_code.hooks.models.adapters import HOOKS_CONFIG_ADAPTER -from deepagents_code.hooks.models.config import HooksConfig, MatcherGroup +from deepagents_code.hooks.models.config import ( + CommandHandlerSpec, + HooksConfig, + MatcherGroup, +) from deepagents_code.hooks.models.domain import HookDiagnostic, HookEvent from deepagents_code.model_config import DEFAULT_CONFIG_DIR logger = logging.getLogger(__name__) +_LEGACY_HOOKS_REMOVAL_DATE = "September 1, 2026" @dataclass(frozen=True, slots=True) @@ -47,16 +51,16 @@ class LoadedHooksConfig: snapshot_id: str -def project_hooks_path(cwd: Path) -> Path: +def project_hooks_path(project_root: Path) -> Path: """Return the project-scoped hooks configuration path. Args: - cwd: Session working directory. + project_root: Project root directory. Returns: - `{cwd}/.deepagents/hooks.json`. + `{project_root}/.deepagents/hooks.json`. """ - return cwd / ".deepagents" / "hooks.json" + return project_root / ".deepagents" / "hooks.json" def user_hooks_path(config_dir: Path | None = None) -> Path: @@ -73,7 +77,7 @@ def user_hooks_path(config_dir: Path | None = None) -> Path: def load_hooks_config( *, - cwd: Path, + project_root: Path, workspace_trusted: bool, config_dir: Path | None = None, paths: Sequence[Path] | None = None, @@ -81,7 +85,7 @@ def load_hooks_config( """Load, validate, merge, and hash Hooks v2 configuration. Args: - cwd: Session working directory used for project precedence. + project_root: Project root used for project precedence. workspace_trusted: Whether project-scoped hooks may be loaded. config_dir: Alternate user config directory. paths: Explicit trusted source paths in precedence order (highest first). @@ -91,15 +95,20 @@ def load_hooks_config( Returns: Frozen load result with canonical `snapshot_id`. """ - sources = ( + configured_sources = ( tuple(paths) if paths is not None else ( - (project_hooks_path(cwd), user_hooks_path(config_dir)) + (project_hooks_path(project_root), user_hooks_path(config_dir)) if workspace_trusted else (user_hooks_path(config_dir),) ) ) + sources = tuple( + dict.fromkeys( + path.expanduser().resolve(strict=False) for path in configured_sources + ) + ) diagnostics: list[HookDiagnostic] = [] merged: dict[HookEvent, list[MatcherGroup]] = {} loaded_paths: list[Path] = [] @@ -186,7 +195,7 @@ def _read_hooks_document( return None, () try: raw = path.read_text(encoding="utf-8") - data = json.loads(raw) + data: object = json.loads(raw) except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: message = f"Failed to read hooks config at {path}: {exc}" logger.warning(message) @@ -210,9 +219,13 @@ def _read_hooks_document( field=str(path), ), ) - legacy_entries = [item for item in hooks if isinstance(item, Mapping)] + legacy_entries: list[dict[str, object]] = [ + {str(key): value for key, value in item.items()} + for item in hooks + if isinstance(item, Mapping) + ] migrated = migrate_legacy_hooks(legacy_entries) - message = ( + migration_message = ( f"Migrated semantically equivalent session.end hooks from {path}; " "all other legacy events remain unmapped" if migrated.hooks @@ -221,25 +234,127 @@ def _read_hooks_document( "migrate to Hooks v2" ) ) - diagnostic = HookDiagnostic( - code="legacy_migrated" if migrated.hooks else "legacy_unmapped", - severity="debug", - message=message, - field=str(path), - ) - return migrated, (diagnostic,) - - try: - config = HOOKS_CONFIG_ADAPTER.validate_python(data) - except ValidationError as exc: - message = f"Invalid hooks config at {path}: {exc.title}" - logger.warning(message) - return None, ( + return migrated, ( HookDiagnostic( - code="invalid_config", + code="legacy_deprecated", severity="warning", - message=message, + message=( + f"Legacy hooks configuration at {path} is deprecated and will " + f"stop being supported on {_LEGACY_HOOKS_REMOVAL_DATE}" + ), + field=str(path), + ), + HookDiagnostic( + code="legacy_migrated" if migrated.hooks else "legacy_unmapped", + severity="warning", + message=migration_message, field=str(path), ), ) - return config, () + + return _validate_hooks_document(data, path) + + +def _validate_hooks_document( + data: object, + path: Path, +) -> tuple[HooksConfig | None, tuple[HookDiagnostic, ...]]: + if not isinstance(data, Mapping): + return None, (_invalid_config(path, "", "expected an object"),) + raw_hooks = data.get("hooks") + if not isinstance(raw_hooks, Mapping): + return None, (_invalid_config(path, "hooks", "expected an object"),) + + hooks: dict[HookEvent, list[MatcherGroup]] = {} + diagnostics: list[HookDiagnostic] = [] + for raw_event, raw_groups in raw_hooks.items(): + event_field = f"hooks.{raw_event}" + if not isinstance(raw_event, str): + diagnostics.append(_invalid_config(path, event_field, "unknown hook event")) + continue + try: + event = HookEvent(raw_event) + except ValueError: + diagnostics.append(_invalid_config(path, event_field, "unknown hook event")) + continue + if not isinstance(raw_groups, list): + diagnostics.append( + _invalid_config(path, event_field, "expected a list of matcher groups") + ) + continue + + groups: list[MatcherGroup] = [] + for group_index, raw_group in enumerate(raw_groups): + group_field = f"{event_field}[{group_index}]" + group, group_diagnostics = _validate_matcher_group( + raw_group, + path, + group_field, + ) + diagnostics.extend(group_diagnostics) + if group is not None: + groups.append(group) + if groups or not raw_groups: + hooks[event] = groups + + if raw_hooks and not hooks: + return None, tuple(diagnostics) + return HooksConfig(hooks=hooks), tuple(diagnostics) + + +def _validate_matcher_group( + data: object, + path: Path, + field: str, +) -> tuple[MatcherGroup | None, tuple[HookDiagnostic, ...]]: + if not isinstance(data, Mapping): + return None, (_invalid_config(path, field, "expected an object"),) + raw_handlers = data.get("hooks") + if not isinstance(raw_handlers, list): + return None, ( + _invalid_config(path, f"{field}.hooks", "expected a list of handlers"), + ) + + handlers: list[CommandHandlerSpec] = [] + diagnostics: list[HookDiagnostic] = [] + for handler_index, raw_handler in enumerate(raw_handlers): + handler_field = f"{field}.hooks[{handler_index}]" + try: + handlers.append(CommandHandlerSpec.model_validate(raw_handler)) + except ValidationError as exc: + diagnostics.append(_validation_error(path, handler_field, exc)) + + if raw_handlers and not handlers: + return None, tuple(diagnostics) + + group_data = dict(data) + group_data["hooks"] = handlers + try: + return MatcherGroup.model_validate(group_data), tuple(diagnostics) + except ValidationError as exc: + diagnostics.append(_validation_error(path, field, exc)) + return None, tuple(diagnostics) + + +def _validation_error( + path: Path, + field: str, + error: ValidationError, +) -> HookDiagnostic: + details = "; ".join( + str(item["msg"]) + for item in error.errors(include_url=False, include_input=False) + ) + return _invalid_config(path, field, details) + + +def _invalid_config(path: Path, field: str, detail: str) -> HookDiagnostic: + location = f"{path}:{field}" if field else str(path) + message = f"Invalid hooks config at {location}: {detail}" + logger.warning(message) + return HookDiagnostic( + code="invalid_config", + severity="warning", + message=message, + field=location, + ) diff --git a/libs/code/deepagents_code/hooks/models/config.py b/libs/code/deepagents_code/hooks/models/config.py index e24a13ae26..7f01420aed 100644 --- a/libs/code/deepagents_code/hooks/models/config.py +++ b/libs/code/deepagents_code/hooks/models/config.py @@ -2,9 +2,9 @@ from __future__ import annotations -from typing import Literal, Self, TypeAlias +from typing import Literal, TypeAlias -from pydantic import BaseModel, ConfigDict, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, field_validator from deepagents_code.hooks.models.domain import ( # ruff:ignore[typing-only-first-party-import] - Pydantic runtime annotation. HookEvent, @@ -27,20 +27,16 @@ class CommandHandlerSpec(_ConfigModel): type: Literal["command"] command: str - timeout: float | None = None + timeout: float | None = Field(default=None, gt=0, allow_inf_nan=False) status_message: str | None = Field(default=None, alias="statusMessage") async_: bool | None = Field(default=None, alias="async") - @model_validator(mode="after") - def _reject_async_commands(self) -> Self: - if self.async_: - msg = "async command hooks are not supported in MVP" + @field_validator("async_", mode="after") + @classmethod + def _normalize_async(cls, value: bool | None) -> None: + if value: + msg = "async command hooks are not yet supported." raise ValueError(msg) - # Normalize explicit `async: false` to omitted so equivalent configs - # share a snapshot hash. - if self.async_ is False: - return self.model_copy(update={"async_": None}) - return self HandlerSpec: TypeAlias = CommandHandlerSpec diff --git a/libs/code/tests/unit_tests/hooks/test_configuration.py b/libs/code/tests/unit_tests/hooks/test_configuration.py index bc4c5787e9..bc7ccdfb96 100644 --- a/libs/code/tests/unit_tests/hooks/test_configuration.py +++ b/libs/code/tests/unit_tests/hooks/test_configuration.py @@ -10,8 +10,6 @@ from deepagents_code.hooks.capabilities import ( DEFAULT_COMMAND_TIMEOUT_SECONDS, - HOOK_EVENT_SPECS, - assert_hook_event_registry_complete, get_event_spec, ) from deepagents_code.hooks.loading import ( @@ -29,8 +27,9 @@ def test_registry_covers_all_hook_events() -> None: - assert_hook_event_registry_complete() - assert set(HOOK_EVENT_SPECS) == set(HookEvent) + specs = {event: get_event_spec(event) for event in HookEvent} + assert set(specs) == set(HookEvent) + assert all(event is spec.event for event, spec in specs.items()) assert ( get_event_spec(HookEvent.SESSION_END).default_timeout_seconds == DEFAULT_COMMAND_TIMEOUT_SECONDS @@ -69,7 +68,7 @@ def test_load_hooks_config_precedence_and_snapshot_hash(tmp_path: Path) -> None: ) untrusted = load_hooks_config( - cwd=project_dir, + project_root=project_dir, workspace_trusted=False, config_dir=user_dir, ) @@ -80,7 +79,7 @@ def test_load_hooks_config_precedence_and_snapshot_hash(tmp_path: Path) -> None: assert untrusted.sources == (user_dir / "hooks.json",) loaded = load_hooks_config( - cwd=project_dir, + project_root=project_dir, workspace_trusted=True, config_dir=user_dir, ) @@ -138,7 +137,7 @@ def test_legacy_migration_only_maps_exact_session_end_semantics( encoding="utf-8", ) loaded = load_hooks_config( - cwd=tmp_path, + project_root=tmp_path, workspace_trusted=False, config_dir=user_dir, ) @@ -146,6 +145,8 @@ def test_legacy_migration_only_maps_exact_session_end_semantics( assert HookEvent.SESSION_START not in loaded.config.hooks assert HookEvent.SESSION_END in loaded.config.hooks assert HookEvent.PRE_TOOL_USE not in loaded.config.hooks + assert loaded.diagnostics[0].code == "legacy_deprecated" + assert "September 1, 2026" in loaded.diagnostics[0].message assert any(item.code == "legacy_migrated" for item in loaded.diagnostics) @@ -159,7 +160,7 @@ def test_invalid_config_is_diagnosed(tmp_path: Path) -> None: ) loaded = load_hooks_config( - cwd=tmp_path, + project_root=tmp_path, workspace_trusted=False, config_dir=config_dir, ) @@ -167,7 +168,59 @@ def test_invalid_config_is_diagnosed(tmp_path: Path) -> None: assert loaded.config.hooks == {} assert loaded.sources == () assert [item.code for item in loaded.diagnostics] == ["invalid_config"] - assert loaded.diagnostics[0].field == str(path) + assert loaded.diagnostics[0].field == f"{path}:hooks.Stop[0].hooks[0]" + + +def test_invalid_handler_does_not_discard_valid_siblings(tmp_path: Path) -> None: + path = tmp_path / "hooks.json" + path.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": "valid"}, + {"type": "http", "url": "https://example.com"}, + ] + } + ] + } + } + ), + encoding="utf-8", + ) + + loaded = load_hooks_config( + project_root=tmp_path, + workspace_trusted=False, + paths=[path], + ) + + handlers = loaded.config.hooks[HookEvent.STOP][0].hooks + assert [handler.command for handler in handlers] == ["valid"] + assert loaded.sources == (path.resolve(),) + assert [item.code for item in loaded.diagnostics] == ["invalid_config"] + assert loaded.diagnostics[0].field == f"{path.resolve()}:hooks.Stop[0].hooks[1]" + + +def test_source_paths_are_canonicalized_and_deduplicated(tmp_path: Path) -> None: + config_dir = tmp_path / "config" + config_dir.mkdir() + path = config_dir / "hooks.json" + path.write_text( + '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"once"}]}]}}', + encoding="utf-8", + ) + + loaded = load_hooks_config( + project_root=tmp_path, + workspace_trusted=False, + paths=[path, config_dir / ".." / "config" / "hooks.json"], + ) + + assert loaded.sources == (path.resolve(),) + assert len(loaded.config.hooks[HookEvent.STOP]) == 1 def test_async_command_config_is_rejected() -> None: @@ -191,6 +244,28 @@ def test_async_command_config_is_rejected() -> None: ) +@pytest.mark.parametrize("timeout", [0, -1, float("inf"), float("-inf"), float("nan")]) +def test_command_timeout_must_be_positive_and_finite(timeout: float) -> None: + with pytest.raises(ValidationError, match="timeout"): + HooksConfig.model_validate( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "echo", + "timeout": timeout, + } + ] + } + ] + } + } + ) + + def test_snapshot_id_is_immutable_and_stable() -> None: config = HooksConfig.model_validate( { From 6ede6a85c0f68e61873c9aabf51a0bfe91613833 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 22 Jul 2026 13:11:01 -0700 Subject: [PATCH 6/9] fix(code): pass workspace trust to hooks runtime Co-authored-by: Cursor --- libs/code/deepagents_code/hooks/runtime.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libs/code/deepagents_code/hooks/runtime.py b/libs/code/deepagents_code/hooks/runtime.py index ec84bd0259..0fd55e721f 100644 --- a/libs/code/deepagents_code/hooks/runtime.py +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -54,6 +54,7 @@ def create( cls, *, cwd: Path, + workspace_trusted: bool = False, config_dir: Path | None = None, transcript_root: Path | None = None, ) -> HooksRuntime: @@ -61,6 +62,7 @@ def create( Args: cwd: Session working directory. + workspace_trusted: Whether project-scoped hooks may be loaded. config_dir: Alternate user config directory for tests. transcript_root: Alternate transcript store root. Defaults to `{cwd}/.deepagents/transcripts`. @@ -68,7 +70,11 @@ def create( Returns: A runtime ready to execute invocations for this session. """ - loaded = load_hooks_config(cwd=cwd, config_dir=config_dir) + loaded = load_hooks_config( + project_root=cwd, + workspace_trusted=workspace_trusted, + config_dir=config_dir, + ) snapshot = HooksSnapshot.from_config( loaded.config, diagnostics=loaded.diagnostics, From 51c5b2b5c7dc0703f3ff43c529e05e180bea37e2 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 22 Jul 2026 15:08:32 -0700 Subject: [PATCH 7/9] cleanup --- libs/code/deepagents_code/hooks/loading.py | 2 +- libs/code/deepagents_code/hooks/reducer.py | 4 ++-- libs/code/deepagents_code/hooks/runner.py | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/libs/code/deepagents_code/hooks/loading.py b/libs/code/deepagents_code/hooks/loading.py index e5af0d2e60..c2791a6454 100644 --- a/libs/code/deepagents_code/hooks/loading.py +++ b/libs/code/deepagents_code/hooks/loading.py @@ -151,7 +151,7 @@ def canonical_hooks_bytes(config: HooksConfig) -> bytes: Returns: UTF-8 JSON with sorted keys, event order fixed to `HookEvent`, and - `None` fields omitted. Unsupported MVP fields such as `async` are + `None` fields omitted. Unsupported fields such as `async` are excluded so equivalent configs hash identically. """ payload = { diff --git a/libs/code/deepagents_code/hooks/reducer.py b/libs/code/deepagents_code/hooks/reducer.py index 797ceab7c5..b3035e421d 100644 --- a/libs/code/deepagents_code/hooks/reducer.py +++ b/libs/code/deepagents_code/hooks/reducer.py @@ -205,7 +205,7 @@ def _merge_block( if policy is ExitCodePolicy.IGNORE: return # DIAGNOSE: exit 2 / decision:"block" is not a veto. SubagentStop still - # retains parent-visible context on the first attempt per the MVP matrix. + # retains parent-visible context on the first attempt. if event is HookEvent.SUBAGENT_STOP: if ( isinstance(invocation.event, SubagentStopEvent) @@ -218,7 +218,7 @@ def _merge_block( code="unsupported_block", severity="warning", message=( - "Blocking SubagentStop is not supported in MVP; " + "Blocking SubagentStop is not supported yet; " f"retained as parent context: {message}" ), handler_id=handler_id, diff --git a/libs/code/deepagents_code/hooks/runner.py b/libs/code/deepagents_code/hooks/runner.py index f9a9479468..8cd6964f58 100644 --- a/libs/code/deepagents_code/hooks/runner.py +++ b/libs/code/deepagents_code/hooks/runner.py @@ -29,7 +29,6 @@ from deepagents_code.hooks.snapshot import HookHandler -DEFAULT_HOOK_TIMEOUT = DEFAULT_COMMAND_TIMEOUT_SECONDS MAX_HOOK_OUTPUT_BYTES = 100_000 _READ_CHUNK_BYTES = 8192 _BLOCKING_EXIT_CODE = 2 @@ -52,7 +51,7 @@ async def run_command_handler( *, cwd: Path, event: HookEvent | None = None, - default_timeout: float = DEFAULT_HOOK_TIMEOUT, + default_timeout: float = DEFAULT_COMMAND_TIMEOUT_SECONDS, max_output_bytes: int = MAX_HOOK_OUTPUT_BYTES, env: dict[str, str] | None = None, ) -> HandlerResult: From d672f6295f213bd5d971e7431c83b6c5cba1709f Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 22 Jul 2026 16:31:39 -0700 Subject: [PATCH 8/9] cr --- libs/code/deepagents_code/config_manifest.py | 3 +- .../deepagents_code/hooks/capabilities.py | 3 +- libs/code/deepagents_code/hooks/engine.py | 11 +- libs/code/deepagents_code/hooks/env.py | 36 +- libs/code/deepagents_code/hooks/projection.py | 348 ++++++++++-------- libs/code/deepagents_code/hooks/reducer.py | 279 ++++++++------ libs/code/deepagents_code/hooks/runner.py | 31 +- libs/code/deepagents_code/hooks/tools.py | 34 ++ ...minal.py => validate_terminal_sequence.py} | 1 - .../tests/unit_tests/hooks/test_engine.py | 40 +- .../tests/unit_tests/hooks/test_execution.py | 78 ++-- 11 files changed, 489 insertions(+), 375 deletions(-) rename libs/code/deepagents_code/hooks/{terminal.py => validate_terminal_sequence.py} (92%) diff --git a/libs/code/deepagents_code/config_manifest.py b/libs/code/deepagents_code/config_manifest.py index d2ddc76f76..b0c0e07af2 100644 --- a/libs/code/deepagents_code/config_manifest.py +++ b/libs/code/deepagents_code/config_manifest.py @@ -800,7 +800,8 @@ def is_provider_package_installed(provider: str) -> bool: def _is_secret_env(name: str) -> bool: """Return whether a credential env var name carries secret material.""" - return any(marker in name for marker in _SECRET_NAME_MARKERS) + upper = name.upper() + return any(marker in upper for marker in _SECRET_NAME_MARKERS) def _credential_options() -> tuple[ConfigOption, ...]: diff --git a/libs/code/deepagents_code/hooks/capabilities.py b/libs/code/deepagents_code/hooks/capabilities.py index 93008986c3..789067b53f 100644 --- a/libs/code/deepagents_code/hooks/capabilities.py +++ b/libs/code/deepagents_code/hooks/capabilities.py @@ -52,6 +52,7 @@ class PlainOutputPolicy(StrEnum): class ExitCodePolicy(StrEnum): """How exit code 2 is interpreted for an event.""" + CONTEXT = "context" DENY = "deny" FEEDBACK = "feedback" CONTINUE_LOOP = "continue_loop" @@ -196,7 +197,7 @@ class HookEventSpec: decision_model=SubagentStopDecision, matcher_field="agent_name", default_timeout_seconds=DEFAULT_COMMAND_TIMEOUT_SECONDS, - exit_code_policy=ExitCodePolicy.DIAGNOSE, + exit_code_policy=ExitCodePolicy.CONTEXT, plain_output_policy=PlainOutputPolicy.IGNORE, aggregation_policy=AggregationPolicy.CONTEXT, supported_handler_types=frozenset({HandlerType.COMMAND}), diff --git a/libs/code/deepagents_code/hooks/engine.py b/libs/code/deepagents_code/hooks/engine.py index 34b6f95630..98e513a8fc 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -8,10 +8,7 @@ from deepagents_code.hooks.capabilities import get_event_spec from deepagents_code.hooks.models.domain import HookDiagnostic -from deepagents_code.hooks.projection import ( - projection_diagnostics, - serialize_hook_input, -) +from deepagents_code.hooks.projection import serialize_hook_input from deepagents_code.hooks.reducer import reduce_hook_results from deepagents_code.hooks.runner import ( MAX_HOOK_OUTPUT_BYTES, @@ -37,7 +34,7 @@ async def run( self, invocation: HookInvocation, *, - transcript_path: Path | None = None, + transcript_path: Path, agent_transcript_path: Path | None = None, ) -> HookDecision: """Execute matching handlers and return a normalized decision. @@ -55,7 +52,6 @@ async def run( The event-specific decision produced by ordered hook reduction. """ match = self.snapshot.match(invocation) - projected_diagnostics = projection_diagnostics(invocation) try: payload = serialize_hook_input( invocation, @@ -74,7 +70,6 @@ async def run( diagnostics=( *self.snapshot.diagnostics, *match.diagnostics, - *projected_diagnostics, diagnostic, ), ) @@ -91,7 +86,6 @@ async def run( handler, payload, cwd=invocation.context.cwd, - event=event, default_timeout=event_default, max_output_bytes=self.max_output_bytes, ) @@ -104,6 +98,5 @@ async def run( diagnostics=( *self.snapshot.diagnostics, *match.diagnostics, - *projected_diagnostics, ), ) diff --git a/libs/code/deepagents_code/hooks/env.py b/libs/code/deepagents_code/hooks/env.py index f5b9ee28f6..4afd4bce6f 100644 --- a/libs/code/deepagents_code/hooks/env.py +++ b/libs/code/deepagents_code/hooks/env.py @@ -5,38 +5,19 @@ import os from typing import TYPE_CHECKING -from deepagents_code.config_manifest import _SECRET_NAME_MARKERS +from deepagents_code.config_manifest import _is_secret_env if TYPE_CHECKING: from collections.abc import Mapping -_OTEL_PREFIX = "OTEL_" - - -def is_secret_env_name(name: str) -> bool: - """Return whether an environment variable name looks like secret material. - - Uses the same credential-name markers as the config manifest, compared - case-insensitively so mixed-case env names are handled consistently. - - Args: - name: Environment variable name. - - Returns: - `True` when the name matches the repository secret-name policy. - """ - upper = name.upper() - return any(marker in upper for marker in _SECRET_NAME_MARKERS) - def sanitize_hook_environ( source: Mapping[str, str] | None = None, ) -> dict[str, str]: """Build an inherited environment safe to pass to hook subprocesses. - Removes OpenTelemetry exporter variables (matching the compatible harness) - and strips values whose names look like secrets. Hooks are user-authored - trusted code, but secret values should not be ambiently available. + Strips values whose names look like secrets. Hooks are user-authored trusted + code, but secret values should not be ambiently available. Args: source: Environment to sanitize. Defaults to `os.environ`. @@ -44,12 +25,5 @@ def sanitize_hook_environ( Returns: A new environment mapping suitable for `asyncio.create_subprocess_shell`. """ - env = dict(os.environ if source is None else source) - sanitized: dict[str, str] = {} - for key, value in env.items(): - if key.startswith(_OTEL_PREFIX): - continue - if is_secret_env_name(key): - continue - sanitized[key] = value - return sanitized + env = os.environ if source is None else source + return {key: value for key, value in env.items() if not _is_secret_env(key)} diff --git a/libs/code/deepagents_code/hooks/projection.py b/libs/code/deepagents_code/hooks/projection.py index 4ec4306ee7..fc8d1bf13e 100644 --- a/libs/code/deepagents_code/hooks/projection.py +++ b/libs/code/deepagents_code/hooks/projection.py @@ -2,6 +2,7 @@ from __future__ import annotations +from functools import singledispatch from typing import TYPE_CHECKING, NotRequired, TypedDict from langchain_core.messages import ToolMessage @@ -9,7 +10,6 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.models.adapters import HOOK_WIRE_INPUT_ADAPTER from deepagents_code.hooks.models.domain import ( - HookDiagnostic, HookEvent, NotificationEvent, PermissionRequestEvent, @@ -46,20 +46,21 @@ from langgraph.types import Command - from deepagents_code.hooks.models.domain import HookInvocation + from deepagents_code.hooks.models.domain import ( + AgentIdentity, + HookDomainEvent, + HookInvocation, + ) from deepagents_code.hooks.models.wire import HookWireInput -class _CoreWireFields(TypedDict): +class _BaseWireFields(TypedDict): session_id: str transcript_path: str cwd: str permission_mode: NotRequired[WirePermissionMode] prompt_id: NotRequired[UUID] effort: NotRequired[Effort] - - -class _CommonWireFields(_CoreWireFields): agent_id: NotRequired[str] agent_type: NotRequired[str] @@ -67,7 +68,7 @@ class _CommonWireFields(_CoreWireFields): def project_hook_input( invocation: HookInvocation, *, - transcript_path: Path | None = None, + transcript_path: Path, agent_transcript_path: Path | None = None, ) -> HookWireInput: """Project a native hook invocation into the compatible wire contract. @@ -79,107 +80,13 @@ def project_hook_input( Returns: A validated event-specific wire input. - - Raises: - TypeError: If the invocation carries an unsupported event model. - ValueError: If a required materialized transcript path is missing. """ - event = invocation.event - if isinstance(event, SessionStartEvent): - result = SessionStartWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.SESSION_START, - source=event.cause, - model=event.model, - ) - elif isinstance(event, SessionEndEvent): - result = SessionEndWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.SESSION_END, - reason=event.cause, - ) - elif isinstance(event, PermissionRequestEvent): - tool_name, tool_input = to_wire_call(event.call) - result = PermissionRequestWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.PERMISSION_REQUEST, - tool_name=tool_name, - tool_input=tool_input, - ) - elif isinstance(event, NotificationEvent): - result = NotificationWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.NOTIFICATION, - message=event.notification.message, - title=event.notification.title, - notification_type=_notification_type(event.notification.type), - ) - elif isinstance(event, PreToolUseEvent): - tool_name, tool_input = to_wire_call(event.call) - result = PreToolUseWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.PRE_TOOL_USE, - tool_name=tool_name, - tool_input=tool_input, - tool_use_id=event.call.id, - ) - elif isinstance(event, PostToolUseEvent): - tool_name, tool_input = to_wire_call(event.call) - result = PostToolUseWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.POST_TOOL_USE, - tool_name=tool_name, - tool_input=tool_input, - tool_response=_tool_result(event.result), - tool_use_id=event.call.id, - duration_ms=event.duration_ms, - ) - elif isinstance(event, StopEvent): - result = StopWireInput( - **_common_fields(invocation, transcript_path), - hook_event_name=HookEvent.STOP, - stop_hook_active=event.continuation_count > 0, - last_assistant_message=event.last_assistant_message, - background_tasks=[ - BackgroundTaskWire.model_validate(task.model_dump()) - for task in event.background_tasks - ], - session_crons=[ - SessionCronWire.model_validate(cron.model_dump()) - for cron in event.session_crons - ], - ) - elif isinstance(event, SubagentStartEvent): - result = SubagentStartWireInput( - **_core_fields(invocation, transcript_path), - hook_event_name=HookEvent.SUBAGENT_START, - agent_id=event.agent.id, - agent_type=event.agent.name, - ) - elif isinstance(event, SubagentStopEvent): - if agent_transcript_path is None: - msg = "SubagentStop requires a materialized agent transcript path" - raise ValueError(msg) - result = SubagentStopWireInput( - **_core_fields(invocation, transcript_path), - hook_event_name=HookEvent.SUBAGENT_STOP, - stop_hook_active=event.continuation_count > 0, - agent_id=event.agent.id, - agent_type=event.agent.name, - agent_transcript_path=str(agent_transcript_path), - last_assistant_message=event.last_assistant_message, - background_tasks=[ - BackgroundTaskWire.model_validate(task.model_dump()) - for task in event.background_tasks - ], - session_crons=[ - SessionCronWire.model_validate(cron.model_dump()) - for cron in event.session_crons - ], - ) - else: - msg = f"Unsupported hook event: {type(event).__name__}" - raise TypeError(msg) + result = _project_event( + invocation.event, + invocation, + transcript_path, + agent_transcript_path, + ) payload = HOOK_WIRE_INPUT_ADAPTER.dump_python( result, @@ -190,45 +97,205 @@ def project_hook_input( return HOOK_WIRE_INPUT_ADAPTER.validate_python(payload) -def _core_fields( +@singledispatch +def _project_event( + event: HookDomainEvent, + _invocation: HookInvocation, + _transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + msg = f"Unsupported hook event: {type(event).__name__}" + raise TypeError(msg) + + +@_project_event.register(SessionStartEvent) +def _project_session_start( + event: SessionStartEvent, invocation: HookInvocation, - transcript_path: Path | None, -) -> _CoreWireFields: - context = invocation.context - if transcript_path is None: - msg = "HookInvocation requires a materialized transcript path" + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + return SessionStartWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.SESSION_START, + source=event.cause, + model=event.model, + ) + + +@_project_event.register(SessionEndEvent) +def _project_session_end( + event: SessionEndEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + return SessionEndWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.SESSION_END, + reason=event.cause, + ) + + +@_project_event.register(PermissionRequestEvent) +def _project_permission_request( + event: PermissionRequestEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + tool_name, tool_input = to_wire_call(event.call) + return PermissionRequestWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.PERMISSION_REQUEST, + tool_name=tool_name, + tool_input=tool_input, + ) + + +@_project_event.register(NotificationEvent) +def _project_notification( + event: NotificationEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + return NotificationWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.NOTIFICATION, + message=event.notification.message, + title=event.notification.title, + notification_type=_notification_type(event.notification.type), + ) + + +@_project_event.register(PreToolUseEvent) +def _project_pre_tool_use( + event: PreToolUseEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + tool_name, tool_input = to_wire_call(event.call) + return PreToolUseWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.PRE_TOOL_USE, + tool_name=tool_name, + tool_input=tool_input, + tool_use_id=event.call.id, + ) + + +@_project_event.register(PostToolUseEvent) +def _project_post_tool_use( + event: PostToolUseEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + tool_name, tool_input = to_wire_call(event.call) + return PostToolUseWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.POST_TOOL_USE, + tool_name=tool_name, + tool_input=tool_input, + tool_response=_tool_result(event.result), + tool_use_id=event.call.id, + duration_ms=event.duration_ms, + ) + + +@_project_event.register(StopEvent) +def _project_stop( + event: StopEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + return StopWireInput( + **_base_fields(invocation, transcript_path), + hook_event_name=HookEvent.STOP, + stop_hook_active=event.continuation_count > 0, + last_assistant_message=event.last_assistant_message, + background_tasks=[ + BackgroundTaskWire.model_validate(task.model_dump()) + for task in event.background_tasks + ], + session_crons=[ + SessionCronWire.model_validate(cron.model_dump()) + for cron in event.session_crons + ], + ) + + +@_project_event.register(SubagentStartEvent) +def _project_subagent_start( + event: SubagentStartEvent, + invocation: HookInvocation, + transcript_path: Path, + _agent_transcript_path: Path | None, +) -> HookWireInput: + return SubagentStartWireInput( + **_base_fields(invocation, transcript_path, agent=event.agent), + hook_event_name=HookEvent.SUBAGENT_START, + ) + + +@_project_event.register(SubagentStopEvent) +def _project_subagent_stop( + event: SubagentStopEvent, + invocation: HookInvocation, + transcript_path: Path, + agent_transcript_path: Path | None, +) -> HookWireInput: + if agent_transcript_path is None: + msg = "SubagentStop requires a materialized agent transcript path" raise ValueError(msg) - fields: _CoreWireFields = { + return SubagentStopWireInput( + **_base_fields(invocation, transcript_path, agent=event.agent), + hook_event_name=HookEvent.SUBAGENT_STOP, + stop_hook_active=event.continuation_count > 0, + agent_transcript_path=str(agent_transcript_path), + last_assistant_message=event.last_assistant_message, + background_tasks=[ + BackgroundTaskWire.model_validate(task.model_dump()) + for task in event.background_tasks + ], + session_crons=[ + SessionCronWire.model_validate(cron.model_dump()) + for cron in event.session_crons + ], + ) + + +def _base_fields( + invocation: HookInvocation, + transcript_path: Path, + *, + agent: AgentIdentity | None = None, +) -> _BaseWireFields: + context = invocation.context + fields: _BaseWireFields = { "session_id": context.thread_id, "transcript_path": str(transcript_path), "cwd": str(context.cwd), } - permission_mode = _permission_mode(context.approval_mode) - if permission_mode is not None: - fields["permission_mode"] = permission_mode + fields["permission_mode"] = _permission_mode(context.approval_mode) if context.prompt_id is not None: fields["prompt_id"] = context.prompt_id if context.effort is not None: fields["effort"] = Effort(level=context.effort) - return fields - - -def _common_fields( - invocation: HookInvocation, - transcript_path: Path | None, -) -> _CommonWireFields: - fields: _CommonWireFields = {**_core_fields(invocation, transcript_path)} - agent = invocation.context.agent - if agent is not None: - fields["agent_id"] = agent.id - fields["agent_type"] = agent.name + identity = agent or context.agent + if identity is not None: + fields["agent_id"] = identity.id + fields["agent_type"] = identity.name return fields def serialize_hook_input( invocation: HookInvocation, *, - transcript_path: Path | None = None, + transcript_path: Path, agent_transcript_path: Path | None = None, ) -> bytes: """Serialize a hook invocation as validated compatible JSON. @@ -252,27 +319,10 @@ def serialize_hook_input( ) -def projection_diagnostics(invocation: HookInvocation) -> tuple[HookDiagnostic, ...]: - """Return visible diagnostics for lossy domain-to-wire projection.""" - if invocation.context.approval_mode is ApprovalMode.AUTO: - return ( - HookDiagnostic( - code="unsupported_permission_mode", - severity="warning", - message=( - "AUTO approval mode has no proven compatible hook permission " - "mode and was omitted" - ), - field="permission_mode", - ), - ) - return () - - -def _permission_mode(mode: ApprovalMode) -> WirePermissionMode | None: +def _permission_mode(mode: ApprovalMode) -> WirePermissionMode: return { ApprovalMode.MANUAL: WirePermissionMode.DEFAULT, - ApprovalMode.AUTO: None, + ApprovalMode.AUTO: WirePermissionMode.AUTO, ApprovalMode.YOLO: WirePermissionMode.BYPASS_PERMISSIONS, }[mode] diff --git a/libs/code/deepagents_code/hooks/reducer.py b/libs/code/deepagents_code/hooks/reducer.py index b3035e421d..66d05f7ed6 100644 --- a/libs/code/deepagents_code/hooks/reducer.py +++ b/libs/code/deepagents_code/hooks/reducer.py @@ -3,13 +3,19 @@ from __future__ import annotations from dataclasses import dataclass, field +from functools import singledispatch from typing import TYPE_CHECKING -from deepagents_code.hooks.capabilities import ExitCodePolicy, get_event_spec +from deepagents_code.hooks.capabilities import ( + ExitCodePolicy, + PlainOutputPolicy, + get_event_spec, +) from deepagents_code.hooks.models.domain import ( HookDecision, HookDiagnostic, HookEvent, + HookInvocation, NotificationDecision, PermissionEffect, PermissionRequestDecision, @@ -24,6 +30,7 @@ SubagentStopEvent, ) from deepagents_code.hooks.models.wire import ( + HookSpecificOutput, PermissionAllow, PermissionRequestSpecificOutput, PostToolUseSpecificOutput, @@ -33,19 +40,18 @@ SubagentStartSpecificOutput, SubagentStopSpecificOutput, ) -from deepagents_code.hooks.terminal import validate_terminal_sequence +from deepagents_code.hooks.validate_terminal_sequence import validate_terminal_sequence if TYPE_CHECKING: from collections.abc import Iterable - from deepagents_code.hooks.models.domain import HookInvocation from deepagents_code.hooks.models.wire import HookWireOutput from deepagents_code.hooks.runner import HandlerResult _PERMISSION_RANK = {"none": 0, "allow": 1, "ask": 2, "deny": 3} MAX_STOP_CONTINUATIONS = 8 -_DEFERRED_SESSION_START_FIELDS = ( +_UNSUPPORTED_SESSION_START_FIELDS = ( ("initial_user_message", "initialUserMessage"), ("session_title", "sessionTitle"), ("watch_paths", "watchPaths"), @@ -85,10 +91,21 @@ def reduce_hook_results( The normalized decision for the invocation event. """ state = _Reduction(diagnostics=list(diagnostics)) + plain_output_policy = get_event_spec(invocation.event.event).plain_output_policy for result in results: state.diagnostics.extend(result.diagnostics) if result.plain_output is not None: - state.context.append(result.plain_output) + if plain_output_policy is PlainOutputPolicy.CONTEXT: + state.context.append(result.plain_output) + else: + state.diagnostics.append( + HookDiagnostic( + code="malformed_json", + severity="warning", + message="Hook output is not valid JSON", + handler_id=result.handler_id, + ) + ) if result.output is not None: _merge_output(invocation, state, result.handler_id, result.output) return _decision(invocation, state) @@ -161,7 +178,7 @@ def _merge_output( ) ) return - _merge_specific(invocation, state, handler_id, specific) + _merge_specific(specific, invocation, state, handler_id) def _diagnose_extra_fields( @@ -204,9 +221,7 @@ def _merge_block( return if policy is ExitCodePolicy.IGNORE: return - # DIAGNOSE: exit 2 / decision:"block" is not a veto. SubagentStop still - # retains parent-visible context on the first attempt. - if event is HookEvent.SUBAGENT_STOP: + if policy is ExitCodePolicy.CONTEXT: if ( isinstance(invocation.event, SubagentStopEvent) and invocation.event.continuation_count @@ -261,117 +276,153 @@ def _apply_stop_continuation( state.feedback.append(message) +@singledispatch def _merge_specific( - invocation: HookInvocation, + specific: HookSpecificOutput, + _invocation: HookInvocation, + _state: _Reduction, + _handler_id: str, +) -> None: + msg = f"Unsupported hook-specific output: {type(specific).__name__}" + raise TypeError(msg) + + +@_merge_specific.register +def _merge_session_start( + specific: SessionStartSpecificOutput, + _invocation: HookInvocation, state: _Reduction, handler_id: str, - specific: object, ) -> None: - if isinstance(specific, SessionStartSpecificOutput): - _append(state.context, specific.additional_context) - for attr, wire_name in _DEFERRED_SESSION_START_FIELDS: - value = getattr(specific, attr) - if value in (None, False, [], ""): - continue - state.diagnostics.append( - HookDiagnostic( - code="unsupported_field", - severity="warning", - message=f"{wire_name} is not supported and was ignored", - handler_id=handler_id, - field=wire_name, - ) - ) - elif isinstance(specific, PreToolUseSpecificOutput): - _append(state.context, specific.additional_context) - behavior = specific.permission_decision - if behavior == "defer": - state.diagnostics.append( - HookDiagnostic( - code="unsupported_field", - severity="warning", - message="permissionDecision defer is not supported and was ignored", - handler_id=handler_id, - field="permissionDecision", - ) - ) + _append(state.context, specific.additional_context) + for attr, wire_name in _UNSUPPORTED_SESSION_START_FIELDS: + value = getattr(specific, attr) + if value not in (None, False, [], ""): + _diagnose_unsupported_field(state, handler_id, wire_name) + + +@_merge_specific.register +def _merge_pre_tool_use( + specific: PreToolUseSpecificOutput, + _invocation: HookInvocation, + state: _Reduction, + handler_id: str, +) -> None: + _append(state.context, specific.additional_context) + behavior = specific.permission_decision + if behavior == "defer": + _diagnose_unsupported_field( + state, + handler_id, + "permissionDecision", + value="defer", + ) + behavior = None + if specific.updated_input is not None: + _diagnose_unsupported_updated_input(state, handler_id) + if behavior in {"allow", "ask"}: behavior = None - if specific.updated_input is not None: + if behavior is not None: + _merge_permission( + state, + PermissionEffect( + behavior=behavior, + reason=specific.permission_decision_reason, + ), + ) + + +@_merge_specific.register +def _merge_permission_request( + specific: PermissionRequestSpecificOutput, + _invocation: HookInvocation, + state: _Reduction, + handler_id: str, +) -> None: + decision = specific.decision + if decision.behavior == "allow": + has_updated_input = ( + isinstance(decision, PermissionAllow) and decision.updated_input is not None + ) + if has_updated_input: _diagnose_unsupported_updated_input(state, handler_id) - # Allow/ask coupled to mutation falls back to normal permission flow; - # deny remains safe without applying the mutated input. - if behavior in {"allow", "ask"}: - behavior = None - if behavior is not None: - _merge_permission( - state, - PermissionEffect( - behavior=behavior, - reason=specific.permission_decision_reason, - ), - ) - elif isinstance(specific, PermissionRequestSpecificOutput): - decision = specific.decision - if decision.behavior == "allow": - has_updated_input = ( - isinstance(decision, PermissionAllow) - and decision.updated_input is not None - ) - if has_updated_input: - _diagnose_unsupported_updated_input(state, handler_id) - if isinstance(decision, PermissionAllow) and decision.updated_permissions: - state.diagnostics.append( - HookDiagnostic( - code="unsupported_field", - severity="warning", - message="updatedPermissions is not supported and was ignored", - handler_id=handler_id, - field="updatedPermissions", - ) - ) - if not has_updated_input: - _merge_permission(state, PermissionEffect(behavior="allow")) - else: - _merge_permission( - state, - PermissionEffect( - behavior="deny", - reason=decision.message, - interrupt=decision.interrupt, - ), - ) - elif isinstance(specific, PostToolUseSpecificOutput): - _append(state.context, specific.additional_context) - if specific.updated_tool_output is not None: - state.diagnostics.append( - HookDiagnostic( - code="unsupported_field", - severity="warning", - message="updatedToolOutput is not supported and was ignored", - handler_id=handler_id, - field="updatedToolOutput", - ) - ) - if specific.updated_mcp_tool_output is not None: - state.diagnostics.append( - HookDiagnostic( - code="unsupported_field", - severity="warning", - message="updatedMCPToolOutput is not supported and was ignored", - handler_id=handler_id, - field="updatedMCPToolOutput", - ) - ) - elif isinstance(specific, StopSpecificOutput): - if specific.additional_context is not None: - _apply_stop_continuation(invocation, state, specific.additional_context) - elif isinstance(specific, SubagentStartSpecificOutput): - _append(state.context, specific.additional_context) - elif ( - isinstance(specific, SubagentStopSpecificOutput) - and specific.additional_context is not None - ): - state.context.append(specific.additional_context) + if isinstance(decision, PermissionAllow) and decision.updated_permissions: + _diagnose_unsupported_field(state, handler_id, "updatedPermissions") + if not has_updated_input: + _merge_permission(state, PermissionEffect(behavior="allow")) + return + _merge_permission( + state, + PermissionEffect( + behavior="deny", + reason=decision.message, + interrupt=decision.interrupt, + ), + ) + + +@_merge_specific.register +def _merge_post_tool_use( + specific: PostToolUseSpecificOutput, + _invocation: HookInvocation, + state: _Reduction, + handler_id: str, +) -> None: + _append(state.context, specific.additional_context) + if specific.updated_tool_output is not None: + _diagnose_unsupported_field(state, handler_id, "updatedToolOutput") + if specific.updated_mcp_tool_output is not None: + _diagnose_unsupported_field(state, handler_id, "updatedMCPToolOutput") + + +@_merge_specific.register +def _merge_stop( + specific: StopSpecificOutput, + invocation: HookInvocation, + state: _Reduction, + _handler_id: str, +) -> None: + if specific.additional_context is not None: + _apply_stop_continuation(invocation, state, specific.additional_context) + + +@_merge_specific.register +def _merge_subagent_start( + specific: SubagentStartSpecificOutput, + _invocation: HookInvocation, + state: _Reduction, + _handler_id: str, +) -> None: + _append(state.context, specific.additional_context) + + +@_merge_specific.register +def _merge_subagent_stop( + specific: SubagentStopSpecificOutput, + _invocation: HookInvocation, + state: _Reduction, + _handler_id: str, +) -> None: + _append(state.context, specific.additional_context) + + +def _diagnose_unsupported_field( + state: _Reduction, + handler_id: str, + field: str, + *, + value: str | None = None, +) -> None: + subject = f"{field} value {value!r}" if value is not None else field + state.diagnostics.append( + HookDiagnostic( + code="unsupported_field", + severity="warning", + message=f"{subject} is not supported and was ignored", + handler_id=handler_id, + field=field, + ) + ) def _diagnose_unsupported_updated_input( diff --git a/libs/code/deepagents_code/hooks/runner.py b/libs/code/deepagents_code/hooks/runner.py index 8cd6964f58..2b2aa7a5f2 100644 --- a/libs/code/deepagents_code/hooks/runner.py +++ b/libs/code/deepagents_code/hooks/runner.py @@ -13,14 +13,10 @@ from pydantic import ValidationError -from deepagents_code.hooks.capabilities import ( - DEFAULT_COMMAND_TIMEOUT_SECONDS, - PlainOutputPolicy, - get_event_spec, -) +from deepagents_code.hooks.capabilities import DEFAULT_COMMAND_TIMEOUT_SECONDS from deepagents_code.hooks.env import sanitize_hook_environ from deepagents_code.hooks.models.adapters import HOOK_WIRE_OUTPUT_ADAPTER -from deepagents_code.hooks.models.domain import HookDiagnostic, HookEvent +from deepagents_code.hooks.models.domain import HookDiagnostic from deepagents_code.hooks.models.wire import HookWireOutput if TYPE_CHECKING: @@ -50,7 +46,6 @@ async def run_command_handler( payload: bytes, *, cwd: Path, - event: HookEvent | None = None, default_timeout: float = DEFAULT_COMMAND_TIMEOUT_SECONDS, max_output_bytes: int = MAX_HOOK_OUTPUT_BYTES, env: dict[str, str] | None = None, @@ -61,7 +56,6 @@ async def run_command_handler( handler: Snapshotted command handler. payload: Validated JSON sent to stdin. cwd: Working directory inherited from the invocation. - event: Event used for plain-output policy. Defaults to `handler.event`. default_timeout: Timeout used when the handler has no override. max_output_bytes: Maximum retained bytes for each output stream. env: Optional environment override. Defaults to a sanitized copy of the @@ -76,7 +70,6 @@ async def run_command_handler( if not handler.command.strip(): return _failure(handler.id, "invalid_command", "Hook command is empty") - resolved_event = event or handler.event try: # Shell form preserves pipes, redirects, globs, and $VAR expansion to # match the compatible command-hook contract (no separate args field). @@ -152,21 +145,15 @@ async def run_command_handler( if not stdout.strip(): return HandlerResult(handler_id=handler.id, diagnostics=tuple(diagnostics)) + plain = _decode(stdout).strip() try: decoded = json.loads(stdout) except (json.JSONDecodeError, UnicodeDecodeError): - plain = _decode(stdout).strip() - policy = get_event_spec(resolved_event).plain_output_policy - if policy is PlainOutputPolicy.CONTEXT and plain: - return HandlerResult( - handler_id=handler.id, - plain_output=plain, - diagnostics=tuple(diagnostics), - ) - diagnostics.append( - _diagnostic(handler.id, "malformed_json", "Hook output is not valid JSON") + return HandlerResult( + handler_id=handler.id, + plain_output=plain, + diagnostics=tuple(diagnostics), ) - return HandlerResult(handler_id=handler.id, diagnostics=tuple(diagnostics)) try: output = HOOK_WIRE_OUTPUT_ADAPTER.validate_python(decoded) except ValidationError as exc: @@ -241,8 +228,6 @@ async def _read_bounded( async def _terminate(process: Process) -> None: """Kill the hook process group, then reap the direct child.""" - if process.returncode is not None: - return if os.name == "posix" and process.pid is not None: try: os.killpg(process.pid, signal.SIGKILL) @@ -251,7 +236,7 @@ async def _terminate(process: Process) -> None: except OSError: with suppress(OSError): process.kill() - else: + elif process.returncode is None: with suppress(OSError): process.kill() with suppress(OSError, TimeoutError): diff --git a/libs/code/deepagents_code/hooks/tools.py b/libs/code/deepagents_code/hooks/tools.py index 4e02ef1c0a..2d62655f22 100644 --- a/libs/code/deepagents_code/hooks/tools.py +++ b/libs/code/deepagents_code/hooks/tools.py @@ -36,10 +36,44 @@ def _edit_input(args: JsonObject) -> JsonObject: return _select(args, "file_path", "old_string", "new_string", "replace_all") +def _read_input(args: JsonObject) -> JsonObject: + result = _select(args, "file_path", "limit") + if "offset" in args: + offset = args["offset"] + result["offset"] = ( + offset + 1 + if isinstance(offset, int) and not isinstance(offset, bool) + else offset + ) + return result + + +def _glob_input(args: JsonObject) -> JsonObject: + return _select(args, "pattern", "path") + + +def _grep_input(args: JsonObject) -> JsonObject: + result = _select(args, "path", "glob", "output_mode") + if "pattern" in args: + pattern = args["pattern"] + result["pattern"] = re.escape(pattern) if isinstance(pattern, str) else pattern + if "max_count" in args: + result["head_limit"] = args["max_count"] + return result + + +def _ls_input(args: JsonObject) -> JsonObject: + return _select(args, "path") + + _NATIVE_TO_WIRE: dict[str, tuple[str, Callable[[JsonObject], JsonObject]]] = { "execute": ("Bash", _bash_input), "write_file": ("Write", _write_input), "edit_file": ("Edit", _edit_input), + "read_file": ("Read", _read_input), + "glob": ("Glob", _glob_input), + "grep": ("Grep", _grep_input), + "ls": ("LS", _ls_input), } _MCP_WIRE_RE = re.compile(r"^mcp__.+__.+$") diff --git a/libs/code/deepagents_code/hooks/terminal.py b/libs/code/deepagents_code/hooks/validate_terminal_sequence.py similarity index 92% rename from libs/code/deepagents_code/hooks/terminal.py rename to libs/code/deepagents_code/hooks/validate_terminal_sequence.py index e0ff1fa848..919662b7f3 100644 --- a/libs/code/deepagents_code/hooks/terminal.py +++ b/libs/code/deepagents_code/hooks/validate_terminal_sequence.py @@ -4,7 +4,6 @@ import re -# OSC 0/1/2/9/99/777 terminated by BEL or ST, plus bare BEL. _ALLOWED_SEQUENCE = re.compile( r"(?:" r"\x1b\](?:0|1|2|9|99|777);[^\x00-\x1f\x7f-\x9f]*(?:\x07|\x1b\\)" diff --git a/libs/code/tests/unit_tests/hooks/test_engine.py b/libs/code/tests/unit_tests/hooks/test_engine.py index 176af399cf..83e4f06f0d 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -491,7 +491,7 @@ def test_projects_native_tool_names_to_wire(tmp_path: Path) -> None: assert payload["tool_response"]["goto"] == [] -def test_projection_rejects_unknown_notification_and_omits_auto_mode( +def test_projection_rejects_unknown_notification_and_projects_auto_mode( tmp_path: Path, ) -> None: unknown = HookInvocation( @@ -531,10 +531,10 @@ def test_projection_rejects_unknown_notification_and_omits_auto_mode( by_alias=True, exclude_none=True, ) - assert "permission_mode" not in payload + assert payload["permission_mode"] == "auto" -async def test_engine_requires_client_materialized_transcript_path( +async def test_engine_accepts_auto_permission_mode( tmp_path: Path, ) -> None: invocation = _invocation( @@ -546,7 +546,6 @@ async def test_engine_requires_client_materialized_transcript_path( ) snapshot = HooksSnapshot.from_config(HooksConfig(hooks={})) - missing = await HookEngine(snapshot).run(invocation) automatic = HookInvocation( context=invocation.context.model_copy( update={"approval_mode": ApprovalMode.AUTO} @@ -558,8 +557,7 @@ async def test_engine_requires_client_materialized_transcript_path( transcript_path=_transcript_path(tmp_path), ) - assert [item.code for item in missing.diagnostics] == ["projection_failed"] - assert [item.code for item in auto.diagnostics] == ["unsupported_permission_mode"] + assert auto.diagnostics == [] @pytest.mark.parametrize( @@ -596,13 +594,13 @@ async def test_engine_requires_client_materialized_transcript_path( ( "read_file", {"file_path": "/tmp/result.txt", "offset": 0, "limit": 100}, - "read_file", - {"file_path": "/tmp/result.txt", "offset": 0, "limit": 100}, + "Read", + {"file_path": "/tmp/result.txt", "offset": 1, "limit": 100}, ), ( "glob", {"pattern": "**/*.py", "path": "/tmp"}, - "glob", + "Glob", {"pattern": "**/*.py", "path": "/tmp"}, ), ( @@ -614,16 +612,16 @@ async def test_engine_requires_client_materialized_transcript_path( "output_mode": "content", "max_count": 20, }, - "grep", + "Grep", { - "pattern": "result.*", + "pattern": "result\\.\\*", "path": "/tmp", "glob": "*.txt", "output_mode": "content", - "max_count": 20, + "head_limit": 20, }, ), - ("ls", {"path": "/tmp"}, "ls", {"path": "/tmp"}), + ("ls", {"path": "/tmp"}, "LS", {"path": "/tmp"}), ("custom", {"value": 1}, "custom", {"value": 1}), ], ) @@ -715,7 +713,6 @@ async def test_runner_session_start_plain_stdout_is_context(tmp_path: Path) -> N handler, b"{}", cwd=tmp_path, - event=HookEvent.SESSION_START, ) assert result.output is None @@ -723,7 +720,7 @@ async def test_runner_session_start_plain_stdout_is_context(tmp_path: Path) -> N assert result.diagnostics == () -async def test_runner_pretool_plain_stdout_is_malformed(tmp_path: Path) -> None: +async def test_reducer_applies_pretool_plain_stdout_policy(tmp_path: Path) -> None: code = "print('not json')" snapshot = HooksSnapshot.from_config( _config( @@ -747,11 +744,20 @@ async def test_runner_pretool_plain_stdout_is_malformed(tmp_path: Path) -> None: handler, b"{}", cwd=tmp_path, - event=HookEvent.PRE_TOOL_USE, ) + invocation = _invocation( + tmp_path, + PreToolUseEvent( + event=HookEvent.PRE_TOOL_USE, + call=ToolCallData(id="call", name="execute", args={}), + ), + ) + decision = reduce_hook_results(invocation, [result]) assert result.output is None - assert [item.code for item in result.diagnostics] == ["malformed_json"] + assert result.plain_output == "not json" + assert result.diagnostics == () + assert [item.code for item in decision.diagnostics] == ["malformed_json"] async def test_runner_turns_exit_two_stderr_into_block(tmp_path: Path) -> None: diff --git a/libs/code/tests/unit_tests/hooks/test_execution.py b/libs/code/tests/unit_tests/hooks/test_execution.py index ac5a5eef55..625c9195d4 100644 --- a/libs/code/tests/unit_tests/hooks/test_execution.py +++ b/libs/code/tests/unit_tests/hooks/test_execution.py @@ -14,7 +14,7 @@ PlainOutputPolicy, get_event_spec, ) -from deepagents_code.hooks.env import is_secret_env_name, sanitize_hook_environ +from deepagents_code.hooks.env import sanitize_hook_environ from deepagents_code.hooks.models.config import HooksConfig from deepagents_code.hooks.models.domain import ( HookContext, @@ -29,8 +29,8 @@ from deepagents_code.hooks.reducer import MAX_STOP_CONTINUATIONS, reduce_hook_results from deepagents_code.hooks.runner import HandlerResult, run_command_handler from deepagents_code.hooks.snapshot import HooksSnapshot -from deepagents_code.hooks.terminal import validate_terminal_sequence from deepagents_code.hooks.tools import format_mcp_wire_name, to_wire_call +from deepagents_code.hooks.validate_terminal_sequence import validate_terminal_sequence if TYPE_CHECKING: from pathlib import Path @@ -116,26 +116,26 @@ def test_reducer_rejects_invalid_terminal_and_deferred_fields( assert "unsupported_field" in codes -def test_sanitized_env_strips_secrets_and_otel( - monkeypatch: pytest.MonkeyPatch, -) -> None: - monkeypatch.setenv("SAFE_PATH", "/tmp") - monkeypatch.setenv("OPENAI_API_KEY", "placeholder") - monkeypatch.setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost") - monkeypatch.setenv("MY_TOKEN", "placeholder") - monkeypatch.setenv("PYTHONPATH", "/opt/lib") - monkeypatch.setenv("HOME", "/home/user") - - env = sanitize_hook_environ() +def test_sanitized_env_strips_secrets_from_injected_source() -> None: + env = sanitize_hook_environ( + { + "SAFE_PATH": "/tmp", + "OPENAI_API_KEY": "placeholder", + "OTEL_EXPORTER_OTLP_ENDPOINT": "http://localhost", + "MY_TOKEN": "placeholder", + "mixed_case_secret": "placeholder", + "PYTHONPATH": "/opt/lib", + "HOME": "/home/user", + } + ) assert env["SAFE_PATH"] == "/tmp" + assert env["OTEL_EXPORTER_OTLP_ENDPOINT"] == "http://localhost" assert env["PYTHONPATH"] == "/opt/lib" assert env["HOME"] == "/home/user" assert "OPENAI_API_KEY" not in env - assert "OTEL_EXPORTER_OTLP_ENDPOINT" not in env assert "MY_TOKEN" not in env - assert is_secret_env_name("ANTHROPIC_API_KEY") - assert is_secret_env_name("openai_api_key") + assert "mixed_case_secret" not in env def test_mcp_tool_mapping_requires_resolved_metadata() -> None: @@ -175,9 +175,9 @@ def test_exit_and_plain_output_policies_match_registry() -> None: @pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX-specific") async def test_runner_kills_process_group_on_timeout(tmp_path: Path) -> None: script = tmp_path / "hook.sh" - grandchild_pid = tmp_path / "grandchild.pid" + side_effect = tmp_path / "survived" script.write_text( - f"#!/bin/sh\nsleep 30 &\necho $! > {grandchild_pid}\nwait\n", + f"#!/bin/sh\n(sleep 0.2; touch {side_effect}) &\nwait\n", encoding="utf-8", ) script.chmod(0o755) @@ -190,18 +190,17 @@ async def test_runner_kills_process_group_on_timeout(tmp_path: Path) -> None: ) assert [item.code for item in result.diagnostics] == ["timeout"] - if grandchild_pid.is_file(): - pid = int(grandchild_pid.read_text(encoding="utf-8").strip()) - with pytest.raises(ProcessLookupError): - os.kill(pid, 0) + await asyncio.sleep(0.3) + assert not side_effect.exists() @pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX-specific") async def test_runner_kills_process_group_on_cancellation(tmp_path: Path) -> None: script = tmp_path / "cancel.sh" - grandchild_pid = tmp_path / "grandchild.pid" + ready = tmp_path / "ready" + side_effect = tmp_path / "survived" script.write_text( - f"#!/bin/sh\nsleep 30 &\necho $! > {grandchild_pid}\nwait\n", + f"#!/bin/sh\ntouch {ready}\n(sleep 0.2; touch {side_effect}) &\nwait\n", encoding="utf-8", ) script.chmod(0o755) @@ -214,14 +213,35 @@ async def test_runner_kills_process_group_on_cancellation(tmp_path: Path) -> Non ) ) for _ in range(50): - if grandchild_pid.is_file(): + if ready.exists(): break await asyncio.sleep(0.01) + assert ready.exists() task.cancel() with pytest.raises(asyncio.CancelledError): await task - if grandchild_pid.is_file(): - pid = int(grandchild_pid.read_text(encoding="utf-8").strip()) - with pytest.raises(ProcessLookupError): - os.kill(pid, 0) + await asyncio.sleep(0.3) + assert not side_effect.exists() + + +@pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX-specific") +async def test_runner_kills_descendants_after_shell_exits(tmp_path: Path) -> None: + script = tmp_path / "exited.sh" + side_effect = tmp_path / "survived" + script.write_text( + f"#!/bin/sh\n(sleep 0.2; touch {side_effect}) &\nexit 0\n", + encoding="utf-8", + ) + script.chmod(0o755) + + result = await run_command_handler( + _handler(str(script), timeout=0.05), + b"{}", + cwd=tmp_path, + default_timeout=0.05, + ) + + assert [item.code for item in result.diagnostics] == ["timeout"] + await asyncio.sleep(0.3) + assert not side_effect.exists() From 99be05d59d4784ef139d6db4bce46573da5397c2 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 22 Jul 2026 19:09:08 -0700 Subject: [PATCH 9/9] fix(code): redact transcript URL path credentials Co-authored-by: Cursor --- libs/code/deepagents_code/hooks/runtime.py | 9 ++++--- libs/code/deepagents_code/hooks/transcript.py | 26 ++++++++++++++++--- .../tests/unit_tests/hooks/test_transcript.py | 17 +++++++++++- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/libs/code/deepagents_code/hooks/runtime.py b/libs/code/deepagents_code/hooks/runtime.py index 0fd55e721f..84e42d07c0 100644 --- a/libs/code/deepagents_code/hooks/runtime.py +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -18,6 +18,7 @@ ) from deepagents_code.hooks.snapshot import HooksSnapshot from deepagents_code.hooks.transcript import TranscriptStore +from deepagents_code.model_config import DEFAULT_CONFIG_DIR if TYPE_CHECKING: from collections.abc import Sequence @@ -65,7 +66,8 @@ def create( workspace_trusted: Whether project-scoped hooks may be loaded. config_dir: Alternate user config directory for tests. transcript_root: Alternate transcript store root. Defaults to - `{cwd}/.deepagents/transcripts`. + `~/.deepagents/transcripts`, or `{config_dir}/transcripts` when + an alternate user configuration directory is provided. Returns: A runtime ready to execute invocations for this session. @@ -80,9 +82,8 @@ def create( diagnostics=loaded.diagnostics, snapshot_id=loaded.snapshot_id, ) - store = TranscriptStore( - transcript_root or (cwd / ".deepagents" / "transcripts") - ) + user_config_dir = config_dir or DEFAULT_CONFIG_DIR + store = TranscriptStore(transcript_root or user_config_dir / "transcripts") engine = HookEngine(snapshot) return cls(snapshot=snapshot, transcripts=store, engine=engine, cwd=cwd) diff --git a/libs/code/deepagents_code/hooks/transcript.py b/libs/code/deepagents_code/hooks/transcript.py index ce46dfff99..7b0fddf062 100644 --- a/libs/code/deepagents_code/hooks/transcript.py +++ b/libs/code/deepagents_code/hooks/transcript.py @@ -35,7 +35,7 @@ ) from pydantic import BaseModel, ConfigDict -from deepagents_code.hooks.env import is_secret_env_name +from deepagents_code.config_manifest import _is_secret_env from deepagents_code.json_types import JSON_VALUE_ADAPTER, JsonValue if TYPE_CHECKING: @@ -77,15 +77,34 @@ class TranscriptRecord(BaseModel): model_config = ConfigDict(extra="forbid") schema_version: Literal[1] = TRANSCRIPT_SCHEMA_VERSION + """Transcript schema version used to interpret this record.""" + sequence: int + """Zero-based position of this record within its transcript.""" + record_id: str + """Message identifier, or a deterministic role-and-sequence fallback.""" + timestamp: str | None = None + """Source message timestamp when one is available.""" + thread_id: str + """Conversation thread that owns this record.""" + agent_id: str | None = None + """Subagent scope for an agent transcript, otherwise `None`.""" + role: Literal["user", "assistant", "tool", "system"] + """Normalized conversation role for the projected message.""" + message_id: str | None = None + """Original LangChain message identifier when one is available.""" + content: JsonValue + """Redacted JSON-compatible message content.""" + name: str | None = None + """Tool or message name when one is available.""" @dataclass(frozen=True, slots=True) @@ -319,7 +338,7 @@ def redact_transcript_value(value: object) -> JsonValue: return { str(key): ( "[redacted]" - if is_secret_env_name(str(key)) + if _is_secret_env(str(key)) else redact_transcript_value(item) ) for key, item in value.items() @@ -348,10 +367,11 @@ def _redact_url(value: str) -> str: if ":" in hostname and not hostname.startswith("["): hostname = f"[{hostname}]" netloc = f"{hostname}:{port}" if port is not None else hostname + path = "/[redacted]" if parsed.path else "" query_items = parse_qsl(parsed.query, keep_blank_values=True) query = urlencode([(key, "[redacted]") for key, _value in query_items]) fragment = "[redacted]" if parsed.fragment else "" - return urlunsplit((parsed.scheme, netloc, parsed.path, query, fragment)) + return urlunsplit((parsed.scheme, netloc, path, query, fragment)) def _write_transcript( diff --git a/libs/code/tests/unit_tests/hooks/test_transcript.py b/libs/code/tests/unit_tests/hooks/test_transcript.py index 75136c5cbb..be0d7420f3 100644 --- a/libs/code/tests/unit_tests/hooks/test_transcript.py +++ b/libs/code/tests/unit_tests/hooks/test_transcript.py @@ -112,16 +112,20 @@ def test_transcript_redaction_covers_tokens_and_urls() -> None: bare_token = "sk-" + ("x" * 24) bearer = "Bearer " + ("y" * 24) url = "https://user:password@example.com/path?access_token=opaque#fragment" - redacted = redact_transcript_value(f"{bare_token} {bearer} {url}") + webhook_secret = "T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX" + webhook = f"https://hooks.slack.com/services/{webhook_secret}" + redacted = redact_transcript_value(f"{bare_token} {bearer} {url} {webhook}") assert isinstance(redacted, str) assert bare_token not in redacted assert bearer not in redacted assert "user:password" not in redacted + assert webhook_secret not in redacted assert "opaque" not in redacted assert "fragment" not in redacted assert redacted.count("[redacted]") >= 2 assert "%5Bredacted%5D" in redacted + assert "https://hooks.slack.com/[redacted]" in redacted def test_transcript_repairs_corrupt_existing_file_permissions(tmp_path: Path) -> None: @@ -179,6 +183,17 @@ def append(index: int) -> None: assert handle.revision == concurrent.revision("thread") +def test_runtime_stores_transcripts_outside_workspace(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + config_dir = tmp_path / "config" + workspace.mkdir() + + runtime = HooksRuntime.create(cwd=workspace, config_dir=config_dir) + + assert runtime.transcripts.root == (config_dir / "transcripts").resolve() + assert not (workspace / ".deepagents").exists() + + async def test_runtime_materializes_paths_and_invokes(tmp_path: Path) -> None: config_dir = tmp_path / "cfg" config_dir.mkdir()