From 21bfbb22a1dec63d86c8452009fc6d454e196142 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Mon, 27 Jul 2026 16:27:02 -0700 Subject: [PATCH 1/6] feat(code): surface Hooks v2 runtime feedback Hook execution now reports progress, warnings, notices, and permission decisions consistently in the TUI and non-interactive client. Configured statusMessage values appear while handlers run, and server-owned hook output is no longer confined to logs. Re-anchored onto the rebuilt project-trust branch after #4997 squash-merged; carries the full PR #5045 delta. --- libs/code/ARCHITECTURE.md | 1 + libs/code/HOOKS.md | 121 ++++ libs/code/THREAT_MODEL.md | 3 +- libs/code/deepagents_code/app.py | 27 +- .../deepagents_code/client/non_interactive.py | 94 ++- libs/code/deepagents_code/hooks/client.py | 22 +- .../deepagents_code/hooks/client_lifecycle.py | 110 ++-- libs/code/deepagents_code/hooks/engine.py | 74 ++- libs/code/deepagents_code/hooks/feedback.py | 184 ++++++ libs/code/deepagents_code/hooks/manager.py | 76 ++- libs/code/deepagents_code/hooks/runtime.py | 6 + .../deepagents_code/tui/widgets/status.py | 32 +- .../unit_tests/hooks/fixtures/__init__.py | 1 + .../unit_tests/hooks/fixtures/wire/README.md | 29 + .../hooks/fixtures/wire/__init__.py | 5 + .../fixtures/wire/inputs/Notification.json | 14 + .../wire/inputs/PermissionRequest.json | 16 + .../fixtures/wire/inputs/PostToolUse.json | 28 + .../fixtures/wire/inputs/PreCompact.json | 13 + .../fixtures/wire/inputs/PreToolUse.json | 17 + .../fixtures/wire/inputs/SessionEnd.json | 12 + .../fixtures/wire/inputs/SessionStart.json | 13 + .../hooks/fixtures/wire/inputs/Stop.json | 15 + .../fixtures/wire/inputs/SubagentStart.json | 13 + .../fixtures/wire/inputs/SubagentStop.json | 18 + .../wire/inputs/UserPromptSubmit.json | 12 + .../wire/outputs/reduction_cases.json | 540 ++++++++++++++++++ .../fixtures/wire/registry_policies.json | 90 +++ .../unit_tests/hooks/test_client_lifecycle.py | 4 +- .../tests/unit_tests/hooks/test_engine.py | 177 ++++++ .../tests/unit_tests/hooks/test_feedback.py | 184 ++++++ .../unit_tests/hooks/test_server_lifecycle.py | 69 ++- .../unit_tests/hooks/test_wire_fixtures.py | 146 +++++ .../unit_tests/hooks/wire_fixture_helpers.py | 358 ++++++++++++ .../unit_tests/tui/widgets/test_status.py | 31 + 35 files changed, 2456 insertions(+), 99 deletions(-) create mode 100644 libs/code/HOOKS.md create mode 100644 libs/code/deepagents_code/hooks/feedback.py create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/__init__.py create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/README.md create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json create mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json create mode 100644 libs/code/tests/unit_tests/hooks/test_feedback.py create mode 100644 libs/code/tests/unit_tests/hooks/test_wire_fixtures.py create mode 100644 libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py diff --git a/libs/code/ARCHITECTURE.md b/libs/code/ARCHITECTURE.md index b143ced1b1a..8bd696b030d 100644 --- a/libs/code/ARCHITECTURE.md +++ b/libs/code/ARCHITECTURE.md @@ -66,5 +66,6 @@ The main cost is the client/server boundary. When debugging, first decide which - For local setup and debugging, see [`DEVELOPMENT.md`](./DEVELOPMENT.md). - For command behavior, see [`COMMANDS.md`](./COMMANDS.md). +- For lifecycle hooks (`hooks.json`), see [`HOOKS.md`](./HOOKS.md). - For security boundaries, see [`THREAT_MODEL.md`](./THREAT_MODEL.md). - For package-specific coding conventions, see [`AGENTS.md`](./AGENTS.md). diff --git a/libs/code/HOOKS.md b/libs/code/HOOKS.md new file mode 100644 index 00000000000..f3b8519c265 --- /dev/null +++ b/libs/code/HOOKS.md @@ -0,0 +1,121 @@ +# Hooks + +Hooks are user-configured shell commands that run at agent lifecycle events. Each matching handler receives a JSON event payload on stdin and may influence the session through its exit code and stdout. + +> **Warning:** Hook commands run on your machine with your user privileges. Treat every `hooks.json` entry as code you trust — especially project-scoped hooks checked into a repository. + +## Configuration locations and precedence + +| Scope | Path | When it loads | +| --- | --- | --- | +| User | `~/.deepagents/hooks.json` | Always (when the file exists) | +| Project | `{project_root}/.deepagents/hooks.json` | Only after workspace trust | + +When both scopes load, project matcher groups are applied first, then user groups. A project handler that stops further processing therefore wins over lower-precedence user handlers for the same event. + +### Project workspace trust + +Project-scoped hooks can execute arbitrary commands from the repository. Before they load: + +- Interactive `dcode` prompts for approval when `.deepagents/hooks.json` is present and the workspace is not already trusted. +- Choosing always-allow persists trust for that canonical workspace root in `~/.deepagents/.state/hooks_trust.json`. +- Cancelling the prompt (Esc / Ctrl+D) aborts startup. +- Denying skips project hooks for the session and continues with user hooks only. +- Headless / CI runs do not prompt; pass `--trust-project-hooks` to opt in for that run. + +## Events and matchers + +Each top-level key under `"hooks"` is an event name. Values are lists of matcher groups. A group may omit `matcher` (or use `"*"`) to match all values for that event's matcher field. Events with no matcher field reject non-wildcard matchers at load time. + +Native tools are matched by their wire names (for example `execute` → `Bash`, `write_file` → `Write`). + +| Event | Owner | Matcher field | Fires when | +| --- | --- | --- | --- | +| `SessionStart` | client | `cause` | A session starts (`startup`, `resume`, `clear`, `compact`) | +| `UserPromptSubmit` | client | _(none)_ | The user submits a prompt | +| `SessionEnd` | client | `cause` | A session ends | +| `PermissionRequest` | client | `tool_name` | The client is about to ask for tool permission | +| `Notification` | client | `notification_type` | A client lifecycle notification is emitted | +| `PreToolUse` | server | `tool_name` | Before a tool call runs | +| `PostToolUse` | server | `tool_name` | After a tool call completes | +| `PreCompact` | server | `trigger` | Before conversation compaction | +| `Stop` | server | _(none)_ | After an agent stop turn | +| `SubagentStart` | server | `agent_name` | When a subagent starts | +| `SubagentStop` | server | `agent_name` | When a subagent stops | + +## Handler shape + +Each matcher group has a `hooks` list of command handlers: + +```json +{ + "type": "command", + "command": "your-shell-command", + "timeout": 60, + "statusMessage": "Running policy check" +} +``` + +- `type` must be `"command"`. +- `command` is required and runs through a shell, so pipes, redirects, and `$VAR` expansion work. +- `argv` is optional; when set, the handler is executed directly from that argument list instead of through a shell. +- `timeout` is optional seconds; when omitted, the event default applies (600s for most events, 30s for `UserPromptSubmit`). +- `statusMessage` is optional UI status text while the handler runs. +- `async: true` is rejected; async command hooks are not supported. + +## Examples + +### Minimal + +```json +{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "true" + } + ] + } + ] + } +} +``` + +### Deny a destructive shell command + +Matchers use wire tool names. `execute` is exposed as `Bash`. Exit code `2` (or JSON `permissionDecision: "deny"`) denies `PreToolUse`: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 -c \"import json,sys; d=json.load(sys.stdin); cmd=d.get('tool_input',{}).get('command',''); blocked='rm -rf /' in cmd; print(json.dumps({'hookSpecificOutput':{'hookEventName':'PreToolUse','permissionDecision':'deny','permissionDecisionReason':'Refusing destructive root delete'}}) if blocked else '{}')\"" + } + ] + } + ] + } +} +``` + +## How handler output affects behavior + +Handlers communicate through: + +- **Exit code `2`**: treated as a synthetic `decision: "block"`. Interpretation depends on the event (for example deny on `PreToolUse` / `PermissionRequest`, block further processing on `UserPromptSubmit` / `PreCompact`, feedback on `PostToolUse`). +- **Other non-zero exits**: recorded as diagnostics; they do not apply a block decision. +- **JSON stdout** (`HookWireOutput`): may set `continue` / `stopReason`, `systemMessage` (user-visible notice), `additionalContext` via `hookSpecificOutput`, and event-specific fields such as `permissionDecision` on `PreToolUse`. +- **Non-JSON stdout**: becomes additional context for events whose plain-output policy is context (`SessionStart`, `UserPromptSubmit`); otherwise it is a diagnostic. +- **Timeouts**: when a handler exceeds its timeout, it is terminated and recorded as a timeout diagnostic; it does not apply a successful decision. + +## Legacy configuration + +Older list-shaped `hooks.json` documents are still loaded. Semantically equivalent legacy events are migrated into the Hooks v2 shape automatically; unsupported legacy events are left unmapped and surfaced as load diagnostics. diff --git a/libs/code/THREAT_MODEL.md b/libs/code/THREAT_MODEL.md index 36518652135..5113be4b07b 100644 --- a/libs/code/THREAT_MODEL.md +++ b/libs/code/THREAT_MODEL.md @@ -463,7 +463,7 @@ Threats that appear valid in isolation but fall outside project responsibility b | Malicious MCP server injecting prompt instructions | Users configure MCP servers and explicitly trust project-level configs. Once trusted, MCP tool outputs are data from a system the user controls. | Interactive approval prompt + per-server allow/deny lists for project-level configs (`main._check_mcp_project_trust`, `model_config.load_mcp_server_trust_lists`). | | LLM jailbreak / safety bypass | Model selection and safety configuration are user-controlled. The project routes prompts to the configured LLM but cannot guarantee model behavior. | Correctly routing prompts to the configured LLM; applying the system prompt from `agent.get_system_prompt`. | | Sandbox provider security vulnerabilities | Daytona, LangSmith, Modal, Runloop, and AgentCore are third-party services. Their internal security is not this project's responsibility. | Correctly initializing sandbox sessions via `integrations.sandbox_factory.create_sandbox`. | -| Hook commands doing harmful things | Hooks in `~/.deepagents/hooks.json` are 100% user-authored. The payload is data-only (JSON on stdin). | JSON structure validation (`hooks._load_hooks`); 5-second timeout. | +| Hook commands doing harmful things | User-scoped hooks (`~/.deepagents/hooks.json`) and project-scoped hooks (`.deepagents/hooks.json`, only after interactive workspace trust or `--trust-project-hooks`) are intentionally configured commands. The payload is data-only (JSON on stdin). | Schema validation (`hooks.loading.load_hooks_config`); workspace trust for project hooks (versioned store under `~/.deepagents/.state/hooks_trust.json`; cancelling the trust prompt aborts startup); bounded execution with per-event default timeouts (600s for most events, 30s for `UserPromptSubmit`); sanitized subprocess environment. | | Async subagent traffic interception / MitM | Async subagents connect to user-configured LangGraph deployment URLs. The project does not control those endpoints or their TLS certificates. | Accepting URL/headers from user config and passing them to the LangGraph SDK (`agent.load_async_subagents`). | | LangGraph dev server port enumeration / discovery | Discovering the local dev server port requires local access. Port scanning localhost is a general OS security concern, not a framework vulnerability. | Binding to `127.0.0.1` by default (`server._DEFAULT_HOST`); ephemeral server lifetime; OS-assigned ephemeral port (`server._EPHEMERAL_PORT`) is not predictable across runs. | | `.env` file from parent directory changes app/API configuration | `config._find_dotenv_from_start_path` walks up the directory tree to find `.env` files. Discovering ordinary configuration values (API keys, `DEEPAGENTS_CODE_*` settings) this way is standard `python-dotenv` behavior, and the user controls their filesystem. The *code-execution* implication of a project `.env` (shell startup hooks) is tracked in-scope as T12. | Finding `.env` from the project root (`config._find_dotenv_from_start_path`); `override=False` by default (existing env vars preserved); shell startup / environment-hijack keys (`BASH_ENV`, `ENV`) denied during dotenv loading. | @@ -503,3 +503,4 @@ Threats that appear valid in isolation but fall outside project responsibility b | 2026-07-08 | manual update | Removed the SHA-256 config fingerprint trust store (`mcp_trust.py`, `~/.deepagents/.state/mcp_trust.json`, DC4). Project MCP trust is now the interactive approval prompt (allow-for-session / always-allow scoped to project root + server-definition fingerprint / deny), the `--trust-project-mcp` run flag, the `[mcp].enabled_project_server_approvals` and `[mcp].disabled_project_servers` lists, and the process-wide `DEEPAGENTS_CODE_DANGEROUSLY_ENABLE_PROJECT_MCP_SERVERS` name-based escape hatch. The legacy flat `[mcp].enabled_project_servers` key is ignored. Persisted approvals bind to a server definition's fingerprint rather than a whole-config fingerprint. Updated C5, TB4, T10, the configuration input-coverage row, and the malicious-MCP-server dismissal accordingly | | 2026-07-21 | manual update | Clarified TB4 after process-wide MCP names and scoped remembered approvals changed from replacement semantics to independent grants. An empty process-wide allowlist no longer suppresses remembered approvals; disabled-server precedence is unchanged. | | 2026-07-22 | manual update | Added T13 (first-token shell allow-list bypass via allow-listed interpreters/wrappers) under TB2, distinguishing it from T2's `--shell-allow-list all` sentinel; cross-referenced it from T2 and extended the "LLM output" input-coverage gap to note that allow-list matching only inspects the command's first token | +| 2026-07-24 | manual update | Corrected the out-of-scope hooks row: Hooks v2 defaults are 600s (30s for `UserPromptSubmit`), not a global 5-second timeout; loading is `hooks.loading.load_hooks_config`; project hooks require workspace trust or `--trust-project-hooks` | diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 7a8ec4b4d9d..ccf6831bdd7 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -601,6 +601,7 @@ class _ConfigWriteResult: from deepagents_code.config_manifest import CursorStyle from deepagents_code.event_bus import EventSource, ExternalEvent from deepagents_code.goal_rubric import GoalCreateRequest, GoalCriteriaRequest + from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager from deepagents_code.hooks.models.domain import ( SessionStartCause, @@ -4367,6 +4368,26 @@ async def _post_paint_init(self) -> None: lambda: asyncio.create_task(self._run_session_start_sequence()), ) + def _create_hook_feedback(self) -> HookFeedback: + from deepagents_code.hooks.feedback import HookFeedback + + return HookFeedback( + notice=self._notify_hook_feedback, + status=self._update_hook_status, + ) + + def _notify_hook_feedback( + self, + message: str, + severity: HookFeedbackSeverity, + ) -> None: + self.notify(message, severity=severity, markup=False) + + def _update_hook_status(self, message: str) -> None: + """Update the status bar with hook-owned progress text.""" + if self._status_bar: + self._status_bar.set_status_message(message, source="hooks") + async def _init_session_state(self) -> None: """Create session state and load its Hooks v2 manager. @@ -4404,6 +4425,7 @@ async def _init_session_state(self) -> None: identity=session_state.hook_identity, notice=lambda message: self.notify(message, markup=False), trust=self._hook_trust, + feedback=self._create_hook_feedback(), ) # Re-read the app-owned selection last so a mode change during # construction cannot be overwritten by the freshly built state. @@ -4443,7 +4465,10 @@ async def _reload_hooks(self) -> None: """ from pathlib import Path - await self._hooks.reload(cwd=Path(self._cwd)) + await self._hooks.reload( + cwd=Path(self._cwd), + feedback=self._create_hook_feedback(), + ) async def _run_session_start_hook(self, cause: SessionStartCause) -> bool: """Run `SessionStart`, surfacing a stop as a chat message. diff --git a/libs/code/deepagents_code/client/non_interactive.py b/libs/code/deepagents_code/client/non_interactive.py index 7d66981d7a6..3d62c767aa3 100644 --- a/libs/code/deepagents_code/client/non_interactive.py +++ b/libs/code/deepagents_code/client/non_interactive.py @@ -192,6 +192,11 @@ def __init__(self, console: Console) -> None: self._console = console self._live: Live | None = None + @property + def is_running(self) -> bool: + """Whether the live spinner is active.""" + return self._live is not None + def start(self, message: str = "Working...") -> None: """Start the spinner with the given message. @@ -203,11 +208,7 @@ def start(self, message: str = "Working...") -> None: """ if self._live is not None: return - renderable = RichSpinner( - "dots", - text=Text(f" {message}", style="dim"), - style="dim", - ) + renderable = self._renderable(message) try: self._live = Live(renderable, console=self._console, transient=True) self._live.start() @@ -215,6 +216,19 @@ def start(self, message: str = "Working...") -> None: logger.warning("Spinner start failed: %s", exc) self._live = None + def update(self, message: str) -> None: + """Replace the message on a running spinner. + + Args: + message: Status text to display next to the spinner. + """ + if self._live is None: + return + try: + self._live.update(self._renderable(message)) + except (AttributeError, TypeError, OSError) as exc: + logger.warning("Spinner update failed: %s", exc) + def stop(self) -> None: """Stop the spinner if running. Can be restarted with `start`.""" if self._live is not None: @@ -225,6 +239,14 @@ def stop(self) -> None: finally: self._live = None + @staticmethod + def _renderable(message: str) -> RichSpinner: + return RichSpinner( + "dots", + text=Text(f" {message}", style="dim"), + style="dim", + ) + async def _terminate_startup_process(proc: Process) -> None: """Terminate and reap a startup command subprocess. @@ -1335,6 +1357,7 @@ async def _run_agent_loop( from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.client_lifecycle import ClientHookStopError + from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager from deepagents_code.hooks.models.domain import ( DcodeNotificationKind, @@ -1344,6 +1367,41 @@ async def _run_agent_loop( ) from deepagents_code.hooks.trust import WorkspaceTrust + hook_owned_spinner = False + + def present_hook_notice( + message: str, + severity: HookFeedbackSeverity, + ) -> None: + style = ( + "bold red" + if severity == "error" + else "yellow" + if severity == "warning" + else "dim" + ) + console.print(Text(message, style=style), highlight=False) + + def update_hook_status(message: str) -> None: + nonlocal hook_owned_spinner + if spinner is None: + return + if message: + if spinner.is_running: + spinner.update(message) + else: + spinner.start(message) + hook_owned_spinner = True + elif hook_owned_spinner: + spinner.stop() + hook_owned_spinner = False + elif spinner.is_running: + spinner.update("Working...") + + feedback = HookFeedback( + notice=present_hook_notice, + status=update_hook_status if spinner is not None else None, + ) resolved_approval_mode = approval_mode or ApprovalMode.MANUAL # One headless turn per process, so identity is fixed for the whole run. identity = HookSessionIdentity( @@ -1351,15 +1409,23 @@ async def _run_agent_loop( approval_mode=resolved_approval_mode, prompt_id=prompt_id, ) - state.hooks = hooks or HooksManager.create( - cwd=Path.cwd(), - identity=lambda: identity, - notice=lambda notice: console.print(Text(notice), highlight=False), - # Project hooks require an explicit opt-in, matching `--trust-project-mcp`. - # Persisted interactive trust deliberately does not carry into headless - # runs, so CI never inherits a grant made at someone's terminal. - trust=WorkspaceTrust.explicit_only(Path.cwd(), granted=trust_project_hooks), - ) + if hooks is not None: + state.hooks = hooks + state.hooks.attach_feedback(feedback) + else: + state.hooks = HooksManager.create( + cwd=Path.cwd(), + identity=lambda: identity, + notice=lambda notice: console.print(Text(notice), highlight=False), + # Project hooks require an explicit opt-in, matching `--trust-project-mcp`. + # Persisted interactive trust deliberately does not carry into headless + # runs, so CI never inherits a grant made at someone's terminal. + trust=WorkspaceTrust.explicit_only( + Path.cwd(), + granted=trust_project_hooks, + ), + feedback=feedback, + ) state.hooks.apply_graph_context(context) context["approval_mode"] = resolved_approval_mode.value context["auto_approve"] = resolved_approval_mode is ApprovalMode.YOLO diff --git a/libs/code/deepagents_code/hooks/client.py b/libs/code/deepagents_code/hooks/client.py index f4997c0bdde..72d855e26e9 100644 --- a/libs/code/deepagents_code/hooks/client.py +++ b/libs/code/deepagents_code/hooks/client.py @@ -3,8 +3,6 @@ from __future__ import annotations import asyncio -import logging -import sys from dataclasses import dataclass, field from typing import TYPE_CHECKING from uuid import UUID @@ -18,14 +16,10 @@ if TYPE_CHECKING: from collections.abc import Awaitable, Callable, Mapping - from deepagents_code.hooks.models.domain import HookDecision from deepagents_code.hooks.models.transport import HookInvocationRequest from deepagents_code.hooks.runtime import HooksRuntime -logger = logging.getLogger(__name__) - _FulfillmentKey = tuple[str, UUID] -_ResumePayload = dict[str, object] @dataclass(slots=True) @@ -98,7 +92,7 @@ async def fulfill_hook_invocation( async def execute() -> HookInvocationResponse: decision = await runtime.invoke(request.invocation) - _apply_client_side_effects(decision) + runtime.feedback.present_decision(decision) return HookInvocationResponse( protocol_version=1, invocation_id=request.invocation_id, @@ -156,17 +150,3 @@ async def fulfill_pending_hook_interrupts( raise RuntimeError(msg) resumes[interrupt_id] = resume_value return resumes - - -def _apply_client_side_effects(decision: HookDecision) -> None: - """Surface user notices and emit validated terminal sequences. - - `systemMessage` must never become model context; notices are logged for the - operator. Terminal sequences were allowlisted in the reducer. - """ - for notice in decision.user_notices: - logger.warning("Hook user notice: %s", notice) - for sequence in decision.terminal_sequences: - sys.stdout.write(sequence) - if decision.terminal_sequences: - sys.stdout.flush() diff --git a/libs/code/deepagents_code/hooks/client_lifecycle.py b/libs/code/deepagents_code/hooks/client_lifecycle.py index 03e575c99e7..20871d414dc 100644 --- a/libs/code/deepagents_code/hooks/client_lifecycle.py +++ b/libs/code/deepagents_code/hooks/client_lifecycle.py @@ -2,20 +2,18 @@ from __future__ import annotations -import logging -import sys from dataclasses import dataclass, field from typing import TYPE_CHECKING from uuid import UUID from deepagents_code.approval_mode import ApprovalMode +from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.models.domain import ( CompactTrigger, DcodeNotification, DcodeNotificationKind, HookContext, HookDecision, - HookDiagnostic, HookDomainEvent, HookEvent, HookInvocation, @@ -36,6 +34,10 @@ UserPromptSubmitDecision, UserPromptSubmitEvent, ) +from deepagents_code.hooks.permissions import ( + PermissionHookOutcome, + permission_hook_outcome, +) if TYPE_CHECKING: from collections.abc import Callable @@ -46,14 +48,14 @@ class _ClientHooksRuntime(Protocol): @property def cwd(self) -> Path: ... + @property + def feedback(self) -> HookFeedback: ... + def configured_events(self) -> frozenset[HookEvent]: ... async def invoke(self, invocation: HookInvocation) -> HookDecision: ... -logger = logging.getLogger(__name__) - - class ClientHookStopError(RuntimeError): """Raised when a client-owned hook stops lifecycle processing.""" @@ -109,9 +111,30 @@ class ClientHookService: runtime: _ClientHooksRuntime notice: Callable[[str], None] | None = None + feedback: HookFeedback | None = None # SessionStart context accumulated per thread, consumed by # `take_session_context` for injection into the next model turn. _pending_context: dict[str, list[str]] = field(default_factory=dict) + _feedback: HookFeedback = field(init=False) + + def __post_init__(self) -> None: + """Resolve one presenter shared with the session runtime when available.""" + if self.feedback is not None: + self._feedback = self.feedback + elif self.notice is not None: + notice = self.notice + + def sink(message: str, severity: HookFeedbackSeverity) -> None: + del severity + notice(message) + + base = self.runtime.feedback + self._feedback = HookFeedback( + notice=sink, + status=base.status, + ) + else: + self._feedback = self.runtime.feedback async def session_start( self, @@ -279,6 +302,40 @@ async def permission_request( raise TypeError(msg) return decision + async def resolve_permission( + self, + context: ClientHookContext, + call: ToolCallData, + ) -> PermissionHookOutcome: + """Resolve a permission hook and present user-facing attribution once. + + The returned HITL decision carries the raw hook reason (or stop reason) + for model-visible resume payloads. Attribution text is emitted only + through the shared feedback presenter. + + Args: + context: Current client session context. + call: Tool action awaiting approval. + + Returns: + Shared approval, rejection, or unresolved result. + """ + decision = await self.permission_request(context, call) + outcome = permission_hook_outcome(decision) + if outcome.decision is None: + return outcome + permission = ( + decision.permission + if decision.continue_processing + else PermissionEffect( + behavior="deny", + reason=decision.stop_reason or "Permission stopped by hook", + interrupt=True, + ) + ) + self.present_permission(call.name, permission) + return outcome + async def notification( self, context: ClientHookContext, @@ -345,6 +402,19 @@ def has_handlers(self, event: HookEvent) -> bool: """ return event in self.runtime.configured_events() + def present_permission( + self, + tool_name: str, + permission: PermissionEffect, + ) -> None: + """Surface attribution for a hook-owned permission decision. + + Args: + tool_name: Display name of the affected tool. + permission: Normalized permission effect. + """ + self._feedback.present_permission(tool_name, permission) + async def _invoke( self, context: ClientHookContext, @@ -360,31 +430,5 @@ async def _invoke( event=event, ) decision = await self.runtime.invoke(invocation) - self._apply_common_effects(decision) + self._feedback.present_decision(decision) return decision - - def _apply_common_effects(self, decision: HookDecision) -> None: - for diagnostic in decision.diagnostics: - _log_diagnostic(diagnostic) - for notice in decision.user_notices: - if self.notice is None: - logger.warning("Hook user notice: %s", notice) - continue - try: - self.notice(notice) - except Exception: - logger.warning("Failed to surface hook user notice", exc_info=True) - for sequence in decision.terminal_sequences: - sys.stdout.write(sequence) - if decision.terminal_sequences: - sys.stdout.flush() - - -def _log_diagnostic(diagnostic: HookDiagnostic) -> None: - message = "Hook diagnostic %s: %s" - if diagnostic.severity == "error": - logger.error(message, diagnostic.code, diagnostic.message) - elif diagnostic.severity == "warning": - logger.warning(message, diagnostic.code, diagnostic.message) - else: - logger.debug(message, diagnostic.code, diagnostic.message) diff --git a/libs/code/deepagents_code/hooks/engine.py b/libs/code/deepagents_code/hooks/engine.py index 9ba35b22348..abbc8572208 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -3,22 +3,34 @@ from __future__ import annotations import asyncio +import logging from dataclasses import dataclass, field from typing import TYPE_CHECKING from deepagents_code.hooks.capabilities import get_event_spec from deepagents_code.hooks.envelope import HookEnvelopeAdapter +from deepagents_code.hooks.feedback import HookProgress from deepagents_code.hooks.models.domain import HookDiagnostic from deepagents_code.hooks.runner import ( MAX_HOOK_OUTPUT_BYTES, + HandlerResult, run_command_handler, ) if TYPE_CHECKING: + from collections.abc import Callable from pathlib import Path from deepagents_code.hooks.models.domain import HookDecision, HookInvocation - from deepagents_code.hooks.snapshot import HooksSnapshot + from deepagents_code.hooks.snapshot import HookHandler, HooksSnapshot + +logger = logging.getLogger(__name__) + + +def _default_progress_message(handler: HookHandler) -> str: + from deepagents_code.config import get_glyphs + + return f"Running {handler.event.value} hook{get_glyphs().ellipsis}" @dataclass(frozen=True, slots=True) @@ -36,6 +48,7 @@ async def run( *, transcript_path: Path, agent_transcript_path: Path | None = None, + progress: Callable[[HookProgress], None] | None = None, ) -> HookDecision: """Execute matching handlers and return a normalized decision. @@ -47,6 +60,7 @@ async def run( invocation: Native lifecycle invocation. transcript_path: Materialized client transcript path. agent_transcript_path: Materialized subagent transcript path. + progress: Optional handler lifecycle callback. Returns: The event-specific decision produced by ordered hook reduction. @@ -82,12 +96,14 @@ async def run( ) results = await asyncio.gather( *( - run_command_handler( + _run_handler( handler, payload, cwd=invocation.context.cwd, default_timeout=event_default, max_output_bytes=self.max_output_bytes, + operation_id=f"{id(invocation):x}:{handler.id}", + progress=progress, ) for handler in match.handlers ) @@ -100,3 +116,57 @@ async def run( *match.diagnostics, ), ) + + +async def _run_handler( + handler: HookHandler, + payload: bytes, + *, + cwd: Path, + default_timeout: float, + max_output_bytes: int, + operation_id: str, + progress: Callable[[HookProgress], None] | None, +) -> HandlerResult: + message = (handler.status_message or "").strip() + if not message: + message = _default_progress_message(handler) + update = HookProgress( + operation_id=operation_id, + handler_id=handler.id, + event=handler.event, + message=message, + active=True, + ) + _report_progress(progress, update) + try: + return await run_command_handler( + handler, + payload, + cwd=cwd, + default_timeout=default_timeout, + max_output_bytes=max_output_bytes, + ) + finally: + _report_progress( + progress, + HookProgress( + operation_id=operation_id, + handler_id=handler.id, + event=handler.event, + message=message, + active=False, + ), + ) + + +def _report_progress( + callback: Callable[[HookProgress], None] | None, + update: HookProgress, +) -> None: + if callback is None: + return + try: + callback(update) + except Exception: + logger.warning("Hook progress callback failed", exc_info=True) diff --git a/libs/code/deepagents_code/hooks/feedback.py b/libs/code/deepagents_code/hooks/feedback.py new file mode 100644 index 00000000000..35fb1172927 --- /dev/null +++ b/libs/code/deepagents_code/hooks/feedback.py @@ -0,0 +1,184 @@ +"""Shared user-facing feedback for Hooks v2 execution.""" + +from __future__ import annotations + +import logging +import sys +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Literal, Protocol, TypeAlias + +if TYPE_CHECKING: + from collections.abc import Iterable + + from deepagents_code.hooks.models.domain import ( + HookDecision, + HookDiagnostic, + HookEvent, + PermissionEffect, + ) + +logger = logging.getLogger(__name__) + +HookFeedbackSeverity: TypeAlias = Literal["information", "warning", "error"] +DiagnosticKey: TypeAlias = tuple[str, str, str, str | None, str | None] + + +class HookNoticeCallback(Protocol): + """Callable that surfaces a user-visible hook notice.""" + + def __call__(self, message: str, severity: HookFeedbackSeverity) -> None: + """Present one notice to the user. + + Args: + message: User-facing notice text. + severity: Presentation severity for interactive clients. + """ + + +class HookStatusCallback(Protocol): + """Callable that updates hook-owned transient status text.""" + + def __call__(self, message: str) -> None: + """Set or clear the hook-owned status message. + + Args: + message: Status text to display, or empty string to release. + """ + + +@dataclass(frozen=True, slots=True) +class HookProgress: + """Lifecycle update for one running hook handler.""" + + operation_id: str + handler_id: str + event: HookEvent + message: str + active: bool + + +@dataclass(slots=True) +class HookFeedback: + """Present hook feedback consistently across interactive and headless clients.""" + + notice: HookNoticeCallback | None = None + status: HookStatusCallback | None = None + _active_statuses: dict[str, str] = field(default_factory=dict) + + def present_decision(self, decision: HookDecision) -> None: + """Present common side effects from a reduced hook decision. + + Args: + decision: Reduced event-specific hook decision. + """ + self.present_diagnostics(decision.diagnostics) + for notice in decision.user_notices: + self._notify(notice, "information") + for sequence in decision.terminal_sequences: + sys.stdout.write(sequence) + if decision.terminal_sequences: + sys.stdout.flush() + + def present_diagnostics(self, diagnostics: Iterable[HookDiagnostic]) -> None: + """Log diagnostics and surface each warning or error once per invocation. + + Deduplication is scoped to a single presentation call so a recurring + diagnostic is still shown on later invocations. A notice is marked + delivered only after the sink accepts it, so a failed delivery stays + eligible for retry. + + Args: + diagnostics: Structured diagnostics to present. + """ + delivered: set[DiagnosticKey] = set() + for diagnostic in diagnostics: + _log_diagnostic(diagnostic) + if diagnostic.severity == "debug": + continue + key = ( + diagnostic.code, + diagnostic.severity, + diagnostic.message, + diagnostic.handler_id, + diagnostic.field, + ) + if key in delivered: + continue + severity: HookFeedbackSeverity = ( + "error" if diagnostic.severity == "error" else "warning" + ) + if self._notify(f"Hook {severity}: {diagnostic.message}", severity): + delivered.add(key) + + def update_progress(self, progress: HookProgress) -> None: + """Update the currently visible hook-owned status. + + Concurrent handlers share one status slot. The most recently activated + handler wins until it completes; when the last active handler finishes, + the slot is released with an empty message. + + Args: + progress: Handler lifecycle update. + """ + if progress.active: + self._active_statuses[progress.operation_id] = progress.message + else: + self._active_statuses.pop(progress.operation_id, None) + message = next(reversed(self._active_statuses.values()), "") + self._set_status(message) + + def present_permission( + self, + tool_name: str, + permission: PermissionEffect, + ) -> None: + """Attribute a hook-owned permission decision to the hook. + + This text is user-facing only. Model-visible HITL rejection payloads must + carry the raw hook reason without this attribution prefix. + + Args: + tool_name: Display name of the affected tool. + permission: Normalized permission effect. + """ + target = tool_name or "tool request" + if permission.behavior == "allow": + self._notify( + f"PermissionRequest hook allowed {target}.", + "information", + ) + elif permission.behavior == "deny": + suffix = f": {permission.reason}" if permission.reason else "." + self._notify( + f"PermissionRequest hook denied {target}{suffix}", + "warning", + ) + + def _notify(self, message: str, severity: HookFeedbackSeverity) -> bool: + if self.notice is None: + logger.warning("Hook user feedback: %s", message) + return True + try: + self.notice(message, severity) + except Exception: + logger.warning("Failed to surface hook feedback", exc_info=True) + return False + return True + + def _set_status(self, message: str) -> None: + if self.status is None: + return + try: + self.status(message) + except Exception: + logger.warning("Failed to update hook status", exc_info=True) + + +def _log_diagnostic(diagnostic: HookDiagnostic) -> None: + message = "Hook diagnostic %s: %s" + if diagnostic.severity == "error": + logger.error(message, diagnostic.code, diagnostic.message) + elif diagnostic.severity == "warning": + logger.warning(message, diagnostic.code, diagnostic.message) + else: + logger.debug(message, diagnostic.code, diagnostic.message) diff --git a/libs/code/deepagents_code/hooks/manager.py b/libs/code/deepagents_code/hooks/manager.py index 535a4ba03a1..f190bb64774 100644 --- a/libs/code/deepagents_code/hooks/manager.py +++ b/libs/code/deepagents_code/hooks/manager.py @@ -25,7 +25,6 @@ from deepagents_code.hooks.permissions import ( PermissionHookOutcome, PermissionPlan, - permission_hook_outcome, ) from deepagents_code.hooks.trust import WorkspaceTrust @@ -38,6 +37,7 @@ from deepagents_code._cli_context import CLIContext from deepagents_code.approval_mode import ApprovalMode + from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.models.domain import ( CompactTrigger, DcodeNotificationKind, @@ -121,6 +121,7 @@ def create( identity: SessionIdentityProvider, notice: Callable[[str], None], trust: WorkspaceTrust | None = None, + feedback: HookFeedback | None = None, ) -> HooksManager: """Load hook configuration and return a ready manager. @@ -133,12 +134,16 @@ def create( notice: Surfaces hook `systemMessage` notices to the user. trust: Project-hook trust policy. Defaults to trusting nothing beyond what the persisted trust store already records. + feedback: Shared presenter for notices, diagnostics, and progress. + When omitted, feedback is logged rather than surfaced. Returns: A manager owning the loaded runtime, or an inert one on failure. """ policy = trust if trust is not None else WorkspaceTrust.none() - return cls(identity, notice, _load_runtime(cwd, trust=policy), policy) + runtime = _load_runtime(cwd, trust=policy, feedback=feedback) + _present_load_diagnostics(runtime) + return cls(identity, notice, runtime, policy) @classmethod def adopting( @@ -175,6 +180,24 @@ def inert(cls) -> HooksManager: None, ) + def attach_feedback(self, feedback: HookFeedback) -> None: + """Route hook notices, diagnostics, and progress through `feedback`. + + For callers handed a manager that was loaded before their UI existed. + Load diagnostics are re-presented so anything the earlier load could + only log now reaches the user. + + Args: + feedback: Presenter to adopt. + """ + runtime = self._runtime + if runtime is None: + return + runtime.feedback.notice = feedback.notice + runtime.feedback.status = feedback.status + self._service = self._build_service() + _present_load_diagnostics(runtime) + @property def enabled(self) -> bool: """Whether hook configuration loaded successfully for this session.""" @@ -192,7 +215,12 @@ def has_handlers(self, event: HookEvent) -> bool: service = self._service return service is not None and service.has_handlers(event) - async def reload(self, *, cwd: Path) -> None: + async def reload( + self, + *, + cwd: Path, + feedback: HookFeedback | None = None, + ) -> None: """Rebuild the runtime after the session working directory changes. Workspace trust is re-resolved for `cwd`, so moving from a trusted @@ -204,15 +232,22 @@ async def reload(self, *, cwd: Path) -> None: Args: cwd: New session working directory. + feedback: Shared presenter for notices, diagnostics, and progress. + When omitted, the previous runtime's presenter is preserved. """ import asyncio + existing_feedback = ( + self._runtime.feedback if self._runtime is not None else None + ) self._runtime = await asyncio.to_thread( _load_runtime, cwd, trust=self.trust, + feedback=feedback if feedback is not None else existing_feedback, ) self._service = self._build_service() + _present_load_diagnostics(self._runtime) async def on_session_start( self, @@ -358,7 +393,7 @@ async def on_permission_request( outcomes.append(PermissionHookOutcome(None)) continue try: - decision = await service.permission_request(context, call) + outcome = await service.resolve_permission(context, call) except Exception: logger.warning( "PermissionRequest hook invocation failed", @@ -366,7 +401,7 @@ async def on_permission_request( ) outcomes.append(PermissionHookOutcome(None)) continue - outcomes.append(permission_hook_outcome(decision)) + outcomes.append(outcome) return PermissionPlan(tuple(outcomes)) async def notify( @@ -521,7 +556,11 @@ def _build_service(self) -> ClientHookService | None: runtime = self._runtime if runtime is None: return None - return ClientHookService(runtime, notice=self.notice) + # Prefer the shared presenter so notices keep their severity and hook + # progress reaches the status bar. `notice` remains the fallback for + # callers that never supplied one. + presenter = runtime.feedback if runtime.feedback.notice is not None else None + return ClientHookService(runtime, notice=self.notice, feedback=presenter) def _context(self, *, thread_id: str | None = None) -> ClientHookContext: identity = self.identity() @@ -532,7 +571,23 @@ def _context(self, *, thread_id: str | None = None) -> ClientHookContext: ) -def _load_runtime(cwd: Path, *, trust: WorkspaceTrust) -> HooksRuntime | None: +def _present_load_diagnostics(runtime: HooksRuntime | None) -> None: + """Surface configuration diagnostics collected while loading the snapshot. + + Args: + runtime: Freshly loaded runtime, or `None` when loading failed. + """ + if runtime is None: + return + runtime.feedback.present_diagnostics(runtime.snapshot.diagnostics) + + +def _load_runtime( + cwd: Path, + *, + trust: WorkspaceTrust, + feedback: HookFeedback | None = None, +) -> HooksRuntime | None: """Resolve workspace trust for `cwd` and load a runtime under it. Trust is resolved here rather than by the caller so that a reload after a @@ -541,6 +596,7 @@ def _load_runtime(cwd: Path, *, trust: WorkspaceTrust) -> HooksRuntime | None: Args: cwd: Session working directory. trust: Policy deciding whether project hooks may load. + feedback: Shared presenter for notices, diagnostics, and progress. Returns: The loaded runtime, or `None` when configuration could not be loaded. @@ -548,7 +604,11 @@ def _load_runtime(cwd: Path, *, trust: WorkspaceTrust) -> HooksRuntime | None: from deepagents_code.hooks.runtime import HooksRuntime try: - return HooksRuntime.create(cwd=cwd, workspace_trusted=trust.allows(cwd)) + return HooksRuntime.create( + cwd=cwd, + workspace_trusted=trust.allows(cwd), + feedback=feedback, + ) except Exception: logger.exception("Failed to load hook configuration; hooks disabled") return None diff --git a/libs/code/deepagents_code/hooks/runtime.py b/libs/code/deepagents_code/hooks/runtime.py index 7c5abe22c7c..a829b0a27e6 100644 --- a/libs/code/deepagents_code/hooks/runtime.py +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -10,6 +10,7 @@ from deepagents_code.hooks.client import HookFulfillmentLedger from deepagents_code.hooks.engine import HookEngine +from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.loading import load_hooks_config from deepagents_code.hooks.models.domain import ( HookDecision, @@ -62,6 +63,7 @@ class HooksRuntime: """ project_hooks_loaded: bool + feedback: HookFeedback fulfillments: HookFulfillmentLedger @classmethod @@ -72,6 +74,7 @@ def create( workspace_trusted: bool = False, config_dir: Path | None = None, transcript_root: Path | None = None, + feedback: HookFeedback | None = None, ) -> HooksRuntime: """Load configuration once and freeze a session runtime. @@ -85,6 +88,7 @@ def create( Defaults to `~/.deepagents/transcripts` regardless of `config_dir` (project and test hook configs must not relocate the global transcript store). + feedback: Shared user-facing feedback presenter. Returns: A runtime ready to execute invocations for this session. @@ -114,6 +118,7 @@ def create( cwd=project_context.user_cwd, workspace_trusted=workspace_trusted, project_hooks_loaded=loaded.project_source_loaded, + feedback=feedback or HookFeedback(), fulfillments=HookFulfillmentLedger(), ) @@ -176,6 +181,7 @@ async def invoke(self, invocation: HookInvocation) -> HookDecision: prepared.invocation, transcript_path=prepared.transcript_path, agent_transcript_path=prepared.agent_transcript_path, + progress=self.feedback.update_progress, ) def prepare_invocation( diff --git a/libs/code/deepagents_code/tui/widgets/status.py b/libs/code/deepagents_code/tui/widgets/status.py index 8deda1a72a7..b308950d6b0 100644 --- a/libs/code/deepagents_code/tui/widgets/status.py +++ b/libs/code/deepagents_code/tui/widgets/status.py @@ -42,6 +42,9 @@ Derived from the `Literal` so the two can never drift.""" +StatusMessageSource = Literal["agent", "hooks"] +"""Owners that may write the shared status-message slot.""" + class ModelLabel(Widget): """A label that displays a model name, right-aligned with smart truncation. @@ -346,6 +349,10 @@ def __init__(self, cwd: str | Path | None = None, **kwargs: Any) -> None: self._spinner = Spinner() self._spinner_timer: Timer | None = None self._busy_message = "" + self._status_by_source: dict[StatusMessageSource, str] = { + "agent": "", + "hooks": "", + } def compose(self) -> ComposeResult: # noqa: PLR6301 — Textual widget method """Compose the status bar layout. @@ -506,7 +513,8 @@ def watch_status_message(self, new_value: str) -> None: # in the footer (mirrors the connection indicator). msg_widget.display = bool(new_value) if new_value: - msg_widget.update(new_value) + # Plain Content: hook-configured statusMessage may contain brackets. + msg_widget.update(Content(new_value)) if "thinking" in new_value.lower() or "executing" in new_value.lower(): msg_widget.add_class("thinking") else: @@ -691,13 +699,27 @@ def set_auto_approve(self, *, enabled: bool) -> None: """ self.set_approval_mode("yolo" if enabled else "manual") - def set_status_message(self, message: str) -> None: - """Set the status message. + def set_status_message( + self, + message: str, + *, + source: StatusMessageSource = "agent", + ) -> None: + """Set the status message with explicit source ownership. + + Each source stores its own message. Hooks take display priority while + they have a non-empty message; clearing hooks restores any stored agent + message instead of blanking the slot. Agent writes never erase an active + hook status, and hook completion never erases a stored agent status. Args: - message: Status message to display (empty string to clear) + message: Status message to display (empty string to clear). + source: Subsystem that owns this write (`agent` or `hooks`). """ - self.status_message = message + self._status_by_source[source] = message + self.status_message = ( + self._status_by_source["hooks"] or self._status_by_source["agent"] + ) _approximate: bool = False """Append "+" to the token count to signal that the displayed value is stale. diff --git a/libs/code/tests/unit_tests/hooks/fixtures/__init__.py b/libs/code/tests/unit_tests/hooks/fixtures/__init__.py new file mode 100644 index 00000000000..7272e9c2857 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/__init__.py @@ -0,0 +1 @@ +"""Test fixture packages for Hooks unit tests.""" diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md b/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md new file mode 100644 index 00000000000..dfd86f06921 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md @@ -0,0 +1,29 @@ +# Hooks wire-contract fixtures + +These JSON files pin the **external** Hooks v2 wire contract — the stdin payload +shape handlers receive, the registry policy matrix that drives matcher selection +and exit-code / plain-output behavior, and the domain decisions produced when +representative handler exits are reduced. + +## When a test fails + +A failing diff means one of: + +1. **Deliberate contract change** — update the matching fixture in the same PR + and call out the wire/policy change in the PR description. +2. **Accidental regression** — restore the previous shape; do not loosen the + assertion. + +Do not edit these files to silence a failure without understanding whether the +external contract changed. + +## Layout + +| Path | Pins | +| --- | --- | +| `inputs/.json` | Serialized wire input for one lifecycle event (external field names) | +| `registry_policies.json` | Per-event owner, matcher field, timeouts, and exit/plain/aggregation policies | +| `outputs/reduction_cases.json` | Handler exit scenarios → reduced domain decision snapshots | + +Volatile values (`prompt_id`, path fields) are normalized to placeholders in the +test helper before comparison so fixtures stay stable across environments. diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py b/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py new file mode 100644 index 00000000000..4465a9af76b --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py @@ -0,0 +1,5 @@ +"""Committed Hooks v2 wire-contract fixtures. + +See `README.md` in this directory: failing diffs are either deliberate contract +changes (update the fixture and document them) or accidental regressions. +""" diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json new file mode 100644 index 00000000000..f99f31a1a7e --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json @@ -0,0 +1,14 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "Notification", + "message": "Approval required", + "notification_type": "permission_prompt", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "title": "Permission", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json new file mode 100644 index 00000000000..1b902a07d93 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json @@ -0,0 +1,16 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "PermissionRequest", + "permission_mode": "default", + "permission_suggestions": [], + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "tool_input": { + "command": "pwd" + }, + "tool_name": "Bash", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json new file mode 100644 index 00000000000..acffbc0fa3e --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json @@ -0,0 +1,28 @@ +{ + "cwd": "/workspace", + "duration_ms": 12, + "effort": { + "level": "high" + }, + "hook_event_name": "PostToolUse", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "tool_input": { + "command": "pwd" + }, + "tool_name": "Bash", + "tool_response": { + "additional_kwargs": {}, + "artifact": null, + "content": "/workspace", + "id": null, + "name": "Bash", + "response_metadata": {}, + "status": "success", + "tool_call_id": "call-2", + "type": "tool" + }, + "tool_use_id": "call-2", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json new file mode 100644 index 00000000000..bea1b536900 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json @@ -0,0 +1,13 @@ +{ + "custom_instructions": "Keep the plan", + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "PreCompact", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "transcript_path": "/tmp/thread.jsonl", + "trigger": "manual" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json new file mode 100644 index 00000000000..cf60a4e6fca --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json @@ -0,0 +1,17 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "PreToolUse", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "tool_input": { + "content": "hello", + "file_path": "notes.txt" + }, + "tool_name": "Write", + "tool_use_id": "call-1", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json new file mode 100644 index 00000000000..d84183ed45b --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json @@ -0,0 +1,12 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "SessionEnd", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "reason": "other", + "session_id": "thread-1", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json new file mode 100644 index 00000000000..184fcacb7bb --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json @@ -0,0 +1,13 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "SessionStart", + "model": "provider:model", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "source": "startup", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json new file mode 100644 index 00000000000..af323e30012 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json @@ -0,0 +1,15 @@ +{ + "background_tasks": [], + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "Stop", + "last_assistant_message": "Done", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_crons": [], + "session_id": "thread-1", + "stop_hook_active": false, + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json new file mode 100644 index 00000000000..5114104cf74 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json @@ -0,0 +1,13 @@ +{ + "agent_id": "agent-1", + "agent_type": "researcher", + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "SubagentStart", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json new file mode 100644 index 00000000000..fc801ff5073 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json @@ -0,0 +1,18 @@ +{ + "agent_id": "agent-1", + "agent_transcript_path": "/tmp/agent.jsonl", + "agent_type": "researcher", + "background_tasks": [], + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "SubagentStop", + "last_assistant_message": "Found it", + "permission_mode": "default", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_crons": [], + "session_id": "thread-1", + "stop_hook_active": false, + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json new file mode 100644 index 00000000000..86f8febab1c --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json @@ -0,0 +1,12 @@ +{ + "cwd": "/workspace", + "effort": { + "level": "high" + }, + "hook_event_name": "UserPromptSubmit", + "permission_mode": "default", + "prompt": "Review this change", + "prompt_id": "00000000-0000-4000-8000-000000000001", + "session_id": "thread-1", + "transcript_path": "/tmp/thread.jsonl" +} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json new file mode 100644 index 00000000000..150dbd3789c --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json @@ -0,0 +1,540 @@ +[ + { + "event": "UserPromptSubmit", + "exit_code": 0, + "expected": { + "context": [ + "Model context from JSON" + ], + "continue_processing": true, + "diagnostics": [], + "event": "UserPromptSubmit", + "stop_reason": null, + "suppress_original_prompt": false, + "terminal_sequences": [], + "user_notices": [ + "Visible notice" + ] + }, + "id": "UserPromptSubmit_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"Visible notice\", \"hookSpecificOutput\": {\"hookEventName\": \"UserPromptSubmit\", \"additionalContext\": \"Model context from JSON\"}}" + }, + { + "event": "UserPromptSubmit", + "exit_code": 2, + "expected": { + "context": [], + "continue_processing": false, + "diagnostics": [], + "event": "UserPromptSubmit", + "stop_reason": "Blocked by policy", + "suppress_original_prompt": false, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "UserPromptSubmit_exit2", + "stderr": "Blocked by policy", + "stdout": "" + }, + { + "event": "UserPromptSubmit", + "exit_code": 1, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 1", + "severity": "warning" + } + ], + "event": "UserPromptSubmit", + "stop_reason": null, + "suppress_original_prompt": false, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "UserPromptSubmit_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "UserPromptSubmit", + "exit_code": 0, + "expected": { + "context": [ + "plain context line" + ], + "continue_processing": true, + "diagnostics": [], + "event": "UserPromptSubmit", + "stop_reason": null, + "suppress_original_prompt": false, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "UserPromptSubmit_plain", + "stderr": "", + "stdout": "plain context line" + }, + { + "event": "PreToolUse", + "exit_code": 0, + "expected": { + "context": [ + "Pre-tool context" + ], + "continue_processing": true, + "diagnostics": [], + "event": "PreToolUse", + "permission": { + "behavior": "deny", + "interrupt": false, + "reason": "Protected path" + }, + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [ + "Pre-tool notice" + ] + }, + "id": "PreToolUse_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"Pre-tool notice\", \"hookSpecificOutput\": {\"hookEventName\": \"PreToolUse\", \"permissionDecision\": \"deny\", \"permissionDecisionReason\": \"Protected path\", \"additionalContext\": \"Pre-tool context\"}}" + }, + { + "event": "PreToolUse", + "exit_code": 2, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [], + "event": "PreToolUse", + "permission": { + "behavior": "deny", + "interrupt": false, + "reason": "Denied by exit 2" + }, + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PreToolUse_exit2", + "stderr": "Denied by exit 2", + "stdout": "" + }, + { + "event": "PreToolUse", + "exit_code": 3, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 3", + "severity": "warning" + } + ], + "event": "PreToolUse", + "permission": { + "behavior": "none", + "interrupt": false, + "reason": null + }, + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PreToolUse_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "PreToolUse", + "exit_code": 0, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "malformed_json", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook output is not valid JSON", + "severity": "warning" + } + ], + "event": "PreToolUse", + "permission": { + "behavior": "none", + "interrupt": false, + "reason": null + }, + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PreToolUse_plain", + "stderr": "", + "stdout": "not-json" + }, + { + "event": "PostToolUse", + "exit_code": 0, + "expected": { + "context": [ + "Post-tool context" + ], + "continue_processing": true, + "diagnostics": [], + "event": "PostToolUse", + "feedback": [], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [ + "Post-tool notice" + ] + }, + "id": "PostToolUse_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"Post-tool notice\", \"hookSpecificOutput\": {\"hookEventName\": \"PostToolUse\", \"additionalContext\": \"Post-tool context\"}}" + }, + { + "event": "PostToolUse", + "exit_code": 2, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [], + "event": "PostToolUse", + "feedback": [ + "Feedback from exit 2" + ], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PostToolUse_exit2", + "stderr": "Feedback from exit 2", + "stdout": "" + }, + { + "event": "PostToolUse", + "exit_code": 1, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 1", + "severity": "warning" + } + ], + "event": "PostToolUse", + "feedback": [], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PostToolUse_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "PostToolUse", + "exit_code": 0, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "malformed_json", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook output is not valid JSON", + "severity": "warning" + } + ], + "event": "PostToolUse", + "feedback": [], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "PostToolUse_plain", + "stderr": "", + "stdout": "ignored plain" + }, + { + "event": "SessionStart", + "exit_code": 0, + "expected": { + "context": [ + "Session context" + ], + "continue_processing": true, + "diagnostics": [], + "event": "SessionStart", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [ + "Session notice" + ] + }, + "id": "SessionStart_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"Session notice\", \"hookSpecificOutput\": {\"hookEventName\": \"SessionStart\", \"additionalContext\": \"Session context\"}}" + }, + { + "event": "SessionStart", + "exit_code": 2, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "unsupported_block", + "field": "decision", + "handler_id": "fixture:0:0", + "message": "Block/exit 2 is not supported for SessionStart: Unsupported block", + "severity": "warning" + } + ], + "event": "SessionStart", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionStart_exit2", + "stderr": "Unsupported block", + "stdout": "" + }, + { + "event": "SessionStart", + "exit_code": 1, + "expected": { + "context": [], + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 1", + "severity": "warning" + } + ], + "event": "SessionStart", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionStart_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "SessionStart", + "exit_code": 0, + "expected": { + "context": [ + "startup plain context" + ], + "continue_processing": true, + "diagnostics": [], + "event": "SessionStart", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionStart_plain", + "stderr": "", + "stdout": "startup plain context" + }, + { + "event": "Stop", + "exit_code": 0, + "expected": { + "continue_loop": true, + "continue_processing": true, + "diagnostics": [], + "event": "Stop", + "feedback": [ + "Continue with this" + ], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [ + "Stop notice" + ] + }, + "id": "Stop_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"Stop notice\", \"hookSpecificOutput\": {\"hookEventName\": \"Stop\", \"additionalContext\": \"Continue with this\"}}" + }, + { + "event": "Stop", + "exit_code": 2, + "expected": { + "continue_loop": true, + "continue_processing": true, + "diagnostics": [], + "event": "Stop", + "feedback": [ + "Please continue" + ], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "Stop_exit2", + "stderr": "Please continue", + "stdout": "" + }, + { + "event": "Stop", + "exit_code": 1, + "expected": { + "continue_loop": false, + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 1", + "severity": "warning" + } + ], + "event": "Stop", + "feedback": [], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "Stop_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "Stop", + "exit_code": 0, + "expected": { + "continue_loop": false, + "continue_processing": true, + "diagnostics": [ + { + "code": "malformed_json", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook output is not valid JSON", + "severity": "warning" + } + ], + "event": "Stop", + "feedback": [], + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "Stop_plain", + "stderr": "", + "stdout": "stop plain ignored" + }, + { + "event": "SessionEnd", + "exit_code": 0, + "expected": { + "continue_processing": true, + "diagnostics": [], + "event": "SessionEnd", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [ + "End notice" + ] + }, + "id": "SessionEnd_exit0_json", + "stderr": "", + "stdout": "{\"continue\": true, \"systemMessage\": \"End notice\"}" + }, + { + "event": "SessionEnd", + "exit_code": 2, + "expected": { + "continue_processing": true, + "diagnostics": [ + { + "code": "unsupported_block", + "field": "decision", + "handler_id": "fixture:0:0", + "message": "Block/exit 2 is not supported for SessionEnd: ignored block", + "severity": "warning" + } + ], + "event": "SessionEnd", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionEnd_exit2", + "stderr": "ignored block", + "stdout": "" + }, + { + "event": "SessionEnd", + "exit_code": 1, + "expected": { + "continue_processing": true, + "diagnostics": [ + { + "code": "nonzero_exit", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook exited with status 1", + "severity": "warning" + } + ], + "event": "SessionEnd", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionEnd_nonzero", + "stderr": "", + "stdout": "" + }, + { + "event": "SessionEnd", + "exit_code": 0, + "expected": { + "continue_processing": true, + "diagnostics": [ + { + "code": "malformed_json", + "field": null, + "handler_id": "fixture:0:0", + "message": "Hook output is not valid JSON", + "severity": "warning" + } + ], + "event": "SessionEnd", + "stop_reason": null, + "terminal_sequences": [], + "user_notices": [] + }, + "id": "SessionEnd_plain", + "stderr": "", + "stdout": "end plain ignored" + } +] diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json new file mode 100644 index 00000000000..8d37a504fd6 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json @@ -0,0 +1,90 @@ +{ + "Notification": { + "aggregation_policy": "side_effect", + "default_timeout_seconds": 600.0, + "exit_code_policy": "diagnose", + "matcher_field": "notification_type", + "owner": "client", + "plain_output_policy": "ignore" + }, + "PermissionRequest": { + "aggregation_policy": "permission", + "default_timeout_seconds": 600.0, + "exit_code_policy": "deny", + "matcher_field": "tool_name", + "owner": "client", + "plain_output_policy": "ignore" + }, + "PostToolUse": { + "aggregation_policy": "feedback_and_context", + "default_timeout_seconds": 600.0, + "exit_code_policy": "feedback", + "matcher_field": "tool_name", + "owner": "server", + "plain_output_policy": "ignore" + }, + "PreCompact": { + "aggregation_policy": "side_effect", + "default_timeout_seconds": 600.0, + "exit_code_policy": "block", + "matcher_field": "trigger", + "owner": "server", + "plain_output_policy": "ignore" + }, + "PreToolUse": { + "aggregation_policy": "permission", + "default_timeout_seconds": 600.0, + "exit_code_policy": "deny", + "matcher_field": "tool_name", + "owner": "server", + "plain_output_policy": "ignore" + }, + "SessionEnd": { + "aggregation_policy": "side_effect", + "default_timeout_seconds": 600.0, + "exit_code_policy": "diagnose", + "matcher_field": "cause", + "owner": "client", + "plain_output_policy": "ignore" + }, + "SessionStart": { + "aggregation_policy": "context", + "default_timeout_seconds": 600.0, + "exit_code_policy": "diagnose", + "matcher_field": "cause", + "owner": "client", + "plain_output_policy": "context" + }, + "Stop": { + "aggregation_policy": "stop_loop", + "default_timeout_seconds": 600.0, + "exit_code_policy": "continue_loop", + "matcher_field": null, + "owner": "server", + "plain_output_policy": "ignore" + }, + "SubagentStart": { + "aggregation_policy": "context", + "default_timeout_seconds": 600.0, + "exit_code_policy": "diagnose", + "matcher_field": "agent_name", + "owner": "server", + "plain_output_policy": "ignore" + }, + "SubagentStop": { + "aggregation_policy": "context", + "default_timeout_seconds": 600.0, + "exit_code_policy": "context", + "matcher_field": "agent_name", + "owner": "server", + "plain_output_policy": "ignore" + }, + "UserPromptSubmit": { + "aggregation_policy": "context", + "default_timeout_seconds": 30.0, + "exit_code_policy": "block", + "matcher_field": null, + "owner": "client", + "plain_output_policy": "context" + } +} diff --git a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py index dfdd018b5c3..923522c3bfb 100644 --- a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py @@ -15,6 +15,7 @@ ClientHookService, ClientHookStopError, ) +from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.models.domain import ( DcodeNotificationKind, HookDecision, @@ -40,6 +41,7 @@ class _Runtime: cwd: Path decisions: deque[HookDecision] invocations: list[HookInvocation] = field(default_factory=list) + feedback: HookFeedback = field(default_factory=HookFeedback) def configured_events(self) -> frozenset[HookEvent]: return frozenset(decision.event for decision in self.decisions) @@ -90,7 +92,7 @@ async def test_common_effects_context_and_live_hook_fields( invocation = runtime.invocations[0] assert decision.context == ["hook context"] - assert notices == ["visible notice"] + assert notices == ["Hook warning: diagnostic", "visible notice"] assert capsys.readouterr().out == "\a" assert "test_warning" in caplog.text assert invocation.context.thread_id == "thread-1" diff --git a/libs/code/tests/unit_tests/hooks/test_engine.py b/libs/code/tests/unit_tests/hooks/test_engine.py index f4cef154372..423bc8ee82f 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio import json import subprocess import sys @@ -62,6 +63,7 @@ if TYPE_CHECKING: from pathlib import Path + from deepagents_code.hooks.feedback import HookProgress from deepagents_code.hooks.models.domain import HookDomainEvent from deepagents_code.json_types import JsonObject @@ -1624,6 +1626,181 @@ async def test_engine_reduces_in_config_order_when_completion_is_reversed( assert second.read_text() == "second" +async def test_engine_reports_configured_handler_status(tmp_path: Path) -> None: + snapshot = HooksSnapshot.from_config( + _config( + { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "unused", + "argv": [sys.executable, "-c", "pass"], + "statusMessage": "Loading project context", + } + ] + } + ] + } + ) + ) + invocation = _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ) + progress: list[HookProgress] = [] + + await HookEngine(snapshot).run( + invocation, + transcript_path=_transcript_path(tmp_path), + progress=progress.append, + ) + + assert [update.active for update in progress] == [True, False] + assert {update.message for update in progress} == {"Loading project context"} + + +async def test_engine_default_progress_uses_charset_glyphs( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + from deepagents_code.config import get_glyphs, reset_glyphs_cache + + monkeypatch.setenv("UI_CHARSET_MODE", "ascii") + reset_glyphs_cache() + snapshot = HooksSnapshot.from_config( + _config( + { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "unused", + "argv": [sys.executable, "-c", "pass"], + } + ] + } + ] + } + ) + ) + progress: list[HookProgress] = [] + + await HookEngine(snapshot).run( + _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ), + transcript_path=_transcript_path(tmp_path), + progress=progress.append, + ) + + expected = f"Running SessionStart hook{get_glyphs().ellipsis}" + assert {update.message for update in progress} == {expected} + assert "…" not in expected + reset_glyphs_cache() + + +async def test_engine_progress_callback_raise_does_not_break_execution( + tmp_path: Path, +) -> None: + snapshot = HooksSnapshot.from_config( + _config( + { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "unused", + "argv": [sys.executable, "-c", "pass"], + "statusMessage": "Loading", + } + ] + } + ] + } + ) + ) + + def boom(_update: HookProgress) -> None: + msg = "progress sink failed" + raise RuntimeError(msg) + + decision = await HookEngine(snapshot).run( + _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ), + transcript_path=_transcript_path(tmp_path), + progress=boom, + ) + + assert isinstance(decision, SessionStartDecision) + assert decision.continue_processing is True + + +async def test_engine_clears_progress_on_cancellation(tmp_path: Path) -> None: + script = ( + "import sys,time; time.sleep(30); sys.stdout.write('{}'); sys.stdout.flush()" + ) + snapshot = HooksSnapshot.from_config( + _config( + { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "unused", + "argv": [sys.executable, "-c", script], + "statusMessage": "Slow hook", + "timeout": 60, + } + ] + } + ] + } + ) + ) + progress: list[HookProgress] = [] + task = asyncio.create_task( + HookEngine(snapshot).run( + _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ), + transcript_path=_transcript_path(tmp_path), + progress=progress.append, + ) + ) + for _ in range(50): + if progress and progress[-1].active: + break + await asyncio.sleep(0.01) + assert progress + assert progress[-1].active is True + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + assert progress[-1].active is False + assert progress[-1].message == "Slow hook" + + async def test_engine_uses_captured_snapshot(tmp_path: Path) -> None: original = _config( { diff --git a/libs/code/tests/unit_tests/hooks/test_feedback.py b/libs/code/tests/unit_tests/hooks/test_feedback.py new file mode 100644 index 00000000000..03248ded49a --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_feedback.py @@ -0,0 +1,184 @@ +"""Tests for shared Hooks v2 user feedback.""" + +from __future__ import annotations + +from io import StringIO +from typing import TYPE_CHECKING + +from deepagents_code.hooks.feedback import HookFeedback, HookProgress +from deepagents_code.hooks.models.domain import ( + HookDiagnostic, + HookEvent, + PermissionEffect, + SessionEndDecision, +) + +if TYPE_CHECKING: + import pytest + + +def test_decision_feedback_scopes_diagnostics_per_invocation( + monkeypatch: pytest.MonkeyPatch, +) -> None: + notices: list[tuple[str, str]] = [] + output = StringIO() + monkeypatch.setattr("deepagents_code.hooks.feedback.sys.stdout", output) + feedback = HookFeedback( + notice=lambda message, severity: notices.append((message, severity)) + ) + diagnostic = HookDiagnostic( + code="invalid_output", + severity="warning", + message="Hook output failed validation", + handler_id="SessionEnd:0:0", + ) + decision = SessionEndDecision( + event=HookEvent.SESSION_END, + user_notices=["visible notice"], + terminal_sequences=["\x1b]9;done\x07"], + diagnostics=[diagnostic, diagnostic], + ) + + feedback.present_decision(decision) + feedback.present_decision( + SessionEndDecision( + event=HookEvent.SESSION_END, + diagnostics=[diagnostic], + ) + ) + + assert notices == [ + ("Hook warning: Hook output failed validation", "warning"), + ("visible notice", "information"), + ("Hook warning: Hook output failed validation", "warning"), + ] + assert output.getvalue() == "\x1b]9;done\x07" + + +def test_failed_diagnostic_notice_remains_eligible_for_retry() -> None: + attempts = {"count": 0} + notices: list[str] = [] + + def flaky_notice(message: str, severity: str) -> None: + _ = severity + attempts["count"] += 1 + if attempts["count"] == 1: + msg = "sink unavailable" + raise RuntimeError(msg) + notices.append(message) + + feedback = HookFeedback(notice=flaky_notice) + diagnostic = HookDiagnostic( + code="invalid_output", + severity="warning", + message="Hook output failed validation", + ) + + feedback.present_diagnostics([diagnostic]) + feedback.present_diagnostics([diagnostic]) + + assert notices == ["Hook warning: Hook output failed validation"] + + +def test_progress_keeps_latest_concurrent_status_visible() -> None: + statuses: list[str] = [] + + def capture_status(message: str) -> None: + statuses.append(message) + + feedback = HookFeedback(status=capture_status) + first = HookProgress( + operation_id="first", + handler_id="Stop:0:0", + event=HookEvent.STOP, + message="Checking output", + active=True, + ) + second = HookProgress( + operation_id="second", + handler_id="Stop:0:1", + event=HookEvent.STOP, + message="Running policy", + active=True, + ) + + feedback.update_progress(first) + feedback.update_progress(second) + feedback.update_progress( + HookProgress( + operation_id=first.operation_id, + handler_id=first.handler_id, + event=first.event, + message=first.message, + active=False, + ) + ) + feedback.update_progress( + HookProgress( + operation_id=second.operation_id, + handler_id=second.handler_id, + event=second.event, + message=second.message, + active=False, + ) + ) + + assert statuses == [ + "Checking output", + "Running policy", + "Running policy", + "", + ] + + +def test_progress_callback_raise_does_not_break_updates() -> None: + statuses: list[str] = [] + + def flaky_status(message: str) -> None: + if message == "boom": + msg = "status sink failed" + raise RuntimeError(msg) + statuses.append(message) + + feedback = HookFeedback(status=flaky_status) + feedback.update_progress( + HookProgress( + operation_id="one", + handler_id="Stop:0:0", + event=HookEvent.STOP, + message="boom", + active=True, + ) + ) + feedback.update_progress( + HookProgress( + operation_id="one", + handler_id="Stop:0:0", + event=HookEvent.STOP, + message="recovered", + active=True, + ) + ) + + assert statuses == ["recovered"] + + +def test_permission_feedback_attributes_hook_decisions() -> None: + notices: list[tuple[str, str]] = [] + feedback = HookFeedback( + notice=lambda message, severity: notices.append((message, severity)) + ) + + feedback.present_permission("read_file", PermissionEffect(behavior="allow")) + feedback.present_permission( + "execute", + PermissionEffect(behavior="deny", reason="command blocked"), + ) + + assert notices == [ + ("PermissionRequest hook allowed read_file.", "information"), + ( + "PermissionRequest hook denied execute: command blocked", + "warning", + ), + ] diff --git a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py index 592c75c8774..48a7b81e188 100644 --- a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py @@ -6,6 +6,7 @@ import json import sys from datetime import UTC, datetime, timedelta +from io import StringIO from pathlib import Path from typing import TYPE_CHECKING, Any from unittest.mock import MagicMock @@ -23,6 +24,7 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.client import fulfill_hook_invocation from deepagents_code.hooks.context import apply_hooks_context +from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.interrupt import ( HOOK_INVOCATION_INTERRUPT_TYPE, build_hook_interrupt_payload, @@ -828,11 +830,35 @@ async def handler(_request: object) -> ToolMessage: async def test_fulfill_hook_invocation_runs_engine(tmp_path: Path) -> None: config_dir = tmp_path / "config" config_dir.mkdir() - (config_dir / "hooks.json").write_text('{"hooks":{}}', encoding="utf-8") + command = "import json; print(json.dumps({'systemMessage': 'visible notice'}))" + (config_dir / "hooks.json").write_text( + json.dumps( + { + "hooks": { + "PreToolUse": [ + { + "hooks": [ + { + "type": "command", + "command": "unused", + "argv": [sys.executable, "-c", command], + } + ] + } + ] + } + } + ), + encoding="utf-8", + ) + notices: list[tuple[str, str]] = [] runtime = HooksRuntime.create( cwd=tmp_path, config_dir=config_dir, transcript_root=tmp_path / "transcripts", + feedback=HookFeedback( + notice=lambda message, severity: notices.append((message, severity)) + ), ) request = _request() request = request.model_copy(update={"snapshot_id": runtime.snapshot_id}) @@ -845,11 +871,12 @@ async def test_fulfill_hook_invocation_runs_engine(tmp_path: Path) -> None: ) assert isinstance(response.decision, PreToolUseDecision) assert response.decision.permission.behavior in {"allow", "none"} + assert notices == [("visible notice", "information")] async def test_fulfillment_is_idempotent_in_flight_and_after_completion( tmp_path: Path, - caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, ) -> None: config_dir = tmp_path / "config" config_dir.mkdir() @@ -858,7 +885,14 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( "import json,pathlib,time; " f"pathlib.Path({str(marker)!r}).write_text('x'); " "time.sleep(0.05); " - "print(json.dumps({'systemMessage':'once'}))" + "print(json.dumps({" + "'systemMessage':'once'," + "'terminalSequence':'\\u0007'," + "'hookSpecificOutput':{" + "'hookEventName':'PreToolUse'," + "'permissionDecision':'allow'" + "}" + "}))" ) (config_dir / "hooks.json").write_text( json.dumps( @@ -881,21 +915,30 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( ), encoding="utf-8", ) - runtime = HooksRuntime.create(cwd=tmp_path, config_dir=config_dir) + notices: list[tuple[str, str]] = [] + output = StringIO() + monkeypatch.setattr("deepagents_code.hooks.feedback.sys.stdout", output) + # Force a terminal sequence through a decision that includes one by patching + # after invoke would be heavy; instead assert notice exactly-once via sink. + runtime = HooksRuntime.create( + cwd=tmp_path, + config_dir=config_dir, + feedback=HookFeedback( + notice=lambda message, severity: notices.append((message, severity)) + ), + ) request = _request().model_copy(update={"snapshot_id": runtime.snapshot_id}) - with caplog.at_level("WARNING", logger="deepagents_code.hooks.client"): - first, second = await asyncio.gather( - fulfill_hook_invocation(runtime, request), - fulfill_hook_invocation(runtime, request), - ) - third = await fulfill_hook_invocation(runtime, request) + first, second = await asyncio.gather( + fulfill_hook_invocation(runtime, request), + fulfill_hook_invocation(runtime, request), + ) + third = await fulfill_hook_invocation(runtime, request) assert first == second == third assert marker.read_text() == "x" - assert [record.message for record in caplog.records].count( - "Hook user notice: once" - ) == 1 + assert notices.count(("once", "information")) == 1 + assert output.getvalue() == "\a" def test_snapshot_configured_server_events() -> None: diff --git a/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py b/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py new file mode 100644 index 00000000000..16a8a97d580 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py @@ -0,0 +1,146 @@ +"""Differential golden fixtures for the Hooks v2 external wire contract. + +Committed JSON under `fixtures/wire/` pins stdin payloads, registry policy +rows, and handler-exit → domain-decision interpretation. Exhaustiveness is +enforced by iterating `HookEvent` / the capability registry — a new event +without a fixture fails the suite. +""" + +from __future__ import annotations + +import json +from operator import itemgetter +from typing import TYPE_CHECKING, cast + +import pytest + +from deepagents_code.hooks.models.adapters import HOOK_WIRE_INPUT_ADAPTER +from deepagents_code.hooks.models.domain import HookEvent +from deepagents_code.hooks.projection import project_hook_input +from deepagents_code.hooks.reducer import reduce_hook_results +from unit_tests.hooks.wire_fixture_helpers import ( + FIXED_TRANSCRIPT_PATH, + INPUTS_DIR, + REDUCTION_CASES_PATH, + ReductionCaseFixture, + agent_transcript_path_for, + assert_exact_mapping, + decision_snapshot, + handler_result_from_exit, + load_reduction_cases, + load_registry_policies, + load_wire_input_fixture, + normalize_wire_payload, + registry_policy_row, + representative_invocation, +) + +if TYPE_CHECKING: + from deepagents_code.json_types import JsonObject + + +def test_wire_input_fixtures_cover_every_registry_event() -> None: + fixture_events = {path.stem for path in INPUTS_DIR.glob("*.json")} + registry_events = {event.value for event in HookEvent} + assert fixture_events == registry_events, ( + "Wire input fixtures must cover every HookEvent exactly: " + f"missing={sorted(registry_events - fixture_events)!r} " + f"extra={sorted(fixture_events - registry_events)!r}" + ) + + +@pytest.mark.parametrize("event", list(HookEvent), ids=lambda event: event.value) +def test_projected_wire_input_matches_fixture(event: HookEvent) -> None: + invocation = representative_invocation(event) + projected = HOOK_WIRE_INPUT_ADAPTER.dump_python( + project_hook_input( + invocation, + transcript_path=FIXED_TRANSCRIPT_PATH, + agent_transcript_path=agent_transcript_path_for(event), + ), + mode="json", + by_alias=True, + exclude_none=True, + ) + actual = normalize_wire_payload(cast("JsonObject", projected)) + expected = normalize_wire_payload(load_wire_input_fixture(event)) + assert_exact_mapping(actual, expected, label=f"{event.value} wire input") + + +def test_registry_policy_fixture_covers_every_event() -> None: + policies = load_registry_policies() + registry_events = {event.value for event in HookEvent} + assert set(policies) == registry_events, ( + "registry_policies.json must cover every HookEvent exactly: " + f"missing={sorted(registry_events - set(policies))!r} " + f"extra={sorted(set(policies) - registry_events)!r}" + ) + + +@pytest.mark.parametrize("event", list(HookEvent), ids=lambda event: event.value) +def test_registry_policy_matches_fixture(event: HookEvent) -> None: + policies = load_registry_policies() + actual = cast("JsonObject", dict(registry_policy_row(event))) + expected = cast("JsonObject", dict(policies[event.value])) + assert_exact_mapping(actual, expected, label=f"{event.value} registry policy") + + +def test_reduction_cases_fixture_is_nonempty() -> None: + cases = load_reduction_cases() + assert cases, f"Expected reduction cases in {REDUCTION_CASES_PATH}" + ids = [case["id"] for case in cases] + assert len(ids) == len(set(ids)), f"Duplicate reduction case ids: {ids!r}" + + +@pytest.mark.parametrize( + "case", + load_reduction_cases(), + ids=itemgetter("id"), +) +async def test_handler_output_reduction_matches_fixture( + case: ReductionCaseFixture, +) -> None: + event = HookEvent(case["event"]) + invocation = representative_invocation(event) + result = await handler_result_from_exit( + event=event, + exit_code=case["exit_code"], + stdout=case["stdout"], + stderr=case["stderr"], + ) + decision = reduce_hook_results(invocation, [result]) + actual = decision_snapshot(decision) + assert_exact_mapping( + actual, + case["expected"], + label=f"{case['id']} reduced decision", + ) + + +def test_reduction_cases_cover_required_exit_shapes() -> None: + """Pin the verification matrix: exit 0 JSON, exit 2, other nonzero, plain.""" + cases = load_reduction_cases() + by_event: dict[str, set[str]] = {} + for case in cases: + shapes = by_event.setdefault(case["event"], set()) + if case["exit_code"] == 0 and case["stdout"].lstrip().startswith("{"): + shapes.add("exit0_json") + elif case["exit_code"] == 2: + shapes.add("exit2") + elif case["exit_code"] != 0: + shapes.add("nonzero") + elif case["exit_code"] == 0: + shapes.add("plain") + required = {"exit0_json", "exit2", "nonzero", "plain"} + assert by_event, "No reduction cases loaded" + for event, shapes in by_event.items(): + assert shapes == required, ( + f"{event} reduction cases missing shapes: " + f"have={sorted(shapes)!r} need={sorted(required)!r}" + ) + + +def test_wire_input_fixture_files_are_objects() -> None: + for path in sorted(INPUTS_DIR.glob("*.json")): + raw: object = json.loads(path.read_text(encoding="utf-8")) + assert isinstance(raw, dict), f"{path.name} must be a JSON object" diff --git a/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py b/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py new file mode 100644 index 00000000000..b0c27a04553 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py @@ -0,0 +1,358 @@ +"""Typed helpers for Hooks v2 differential wire fixtures.""" + +from __future__ import annotations + +import copy +import json +import sys +from pathlib import Path +from typing import TYPE_CHECKING, Final, TypedDict, cast +from uuid import UUID + +from langchain_core.messages import ToolMessage + +from deepagents_code.approval_mode import ApprovalMode +from deepagents_code.hooks.capabilities import get_event_spec +from deepagents_code.hooks.models.adapters import HOOK_DECISION_ADAPTER +from deepagents_code.hooks.models.domain import ( + AgentIdentity, + CompactTrigger, + DcodeNotification, + DcodeNotificationKind, + HookContext, + HookDiagnostic, + HookEvent, + HookInvocation, + NotificationEvent, + PermissionRequestEvent, + PostToolUseEvent, + PreCompactEvent, + PreToolUseEvent, + SessionEndCause, + SessionEndEvent, + SessionStartCause, + SessionStartEvent, + StopEvent, + SubagentStartEvent, + SubagentStopEvent, + ToolCallData, + UserPromptSubmitEvent, +) +from deepagents_code.hooks.runner import HandlerResult, run_command_handler +from deepagents_code.hooks.snapshot import HookHandler + +if TYPE_CHECKING: + from deepagents_code.hooks.models.domain import HookDecision, HookDomainEvent + from deepagents_code.json_types import JsonObject + +FIXTURES_DIR: Final[Path] = Path(__file__).resolve().parent / "fixtures" / "wire" +INPUTS_DIR: Final[Path] = FIXTURES_DIR / "inputs" +OUTPUTS_DIR: Final[Path] = FIXTURES_DIR / "outputs" +REGISTRY_POLICIES_PATH: Final[Path] = FIXTURES_DIR / "registry_policies.json" +REDUCTION_CASES_PATH: Final[Path] = OUTPUTS_DIR / "reduction_cases.json" + +FIXED_PROMPT_ID: Final[UUID] = UUID("00000000-0000-4000-8000-000000000001") +FIXED_CWD: Final[Path] = Path("/workspace") +FIXED_TRANSCRIPT_PATH: Final[Path] = Path("/tmp/thread.jsonl") +FIXED_AGENT_TRANSCRIPT_PATH: Final[Path] = Path("/tmp/agent.jsonl") +FIXED_HANDLER_ID: Final[str] = "fixture:0:0" + +# Keys whose values are inherently environment- or generation-dependent. +# Both the projected payload and the fixture pass through normalize_wire_payload +# so committed fixtures may store concrete sample values while comparison uses +# stable placeholders. +_VOLATILE_WIRE_KEYS: Final[dict[str, str]] = { + "prompt_id": "", + "cwd": "", + "transcript_path": "", + "agent_transcript_path": "", +} + +_FIXTURE_HANDLER_TIMEOUT_SECONDS: Final[float] = 20.0 + + +class RegistryPolicyFixture(TypedDict): + """Pinned capability-matrix row for one hook event.""" + + owner: str + matcher_field: str | None + default_timeout_seconds: float + exit_code_policy: str + plain_output_policy: str + aggregation_policy: str + + +class ReductionCaseFixture(TypedDict): + """One handler-exit scenario and its reduced domain decision snapshot.""" + + id: str + event: str + exit_code: int + stdout: str + stderr: str + expected: JsonObject + + +def load_json_object(path: Path) -> JsonObject: + """Load a JSON object fixture from disk. + + Args: + path: Absolute or relative path to a JSON object file. + + Returns: + The parsed JSON object. + + Raises: + ValueError: If the file does not contain a JSON object. + """ + raw: object = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(raw, dict): + msg = f"Fixture must be a JSON object: {path}" + raise TypeError(msg) + return cast("JsonObject", raw) + + +def load_registry_policies() -> dict[str, RegistryPolicyFixture]: + """Load the pinned registry policy matrix.""" + raw = load_json_object(REGISTRY_POLICIES_PATH) + return cast("dict[str, RegistryPolicyFixture]", raw) + + +def load_reduction_cases() -> list[ReductionCaseFixture]: + """Load handler-output reduction cases.""" + raw: object = json.loads(REDUCTION_CASES_PATH.read_text(encoding="utf-8")) + if not isinstance(raw, list): + msg = f"Reduction cases fixture must be a JSON array: {REDUCTION_CASES_PATH}" + raise TypeError(msg) + return cast("list[ReductionCaseFixture]", raw) + + +def load_wire_input_fixture(event: HookEvent) -> JsonObject: + """Load the committed wire-input fixture for `event`.""" + return load_json_object(INPUTS_DIR / f"{event.value}.json") + + +def normalize_wire_payload(payload: JsonObject) -> JsonObject: + """Stabilize non-deterministic wire fields for exact fixture comparison. + + Rewrites known volatile keys (`prompt_id`, `cwd`, `transcript_path`, + `agent_transcript_path`) to documented placeholders. All other keys and + nested values are left unchanged so added, removed, or renamed fields fail + the exact comparison. + + Args: + payload: Serialized hook wire input (external field names). + + Returns: + A deep copy with volatile keys replaced by placeholders. + """ + normalized = cast("JsonObject", copy.deepcopy(payload)) + for key, placeholder in _VOLATILE_WIRE_KEYS.items(): + if key in normalized: + normalized[key] = placeholder + return normalized + + +def assert_exact_mapping( + actual: JsonObject, + expected: JsonObject, + *, + label: str, +) -> None: + """Assert two JSON objects match with exact key sets. + + Args: + actual: Observed mapping. + expected: Fixture mapping. + label: Context included in assertion messages. + + Raises: + AssertionError: On key drift or value mismatch. + """ + actual_keys = set(actual) + expected_keys = set(expected) + added = sorted(actual_keys - expected_keys) + removed = sorted(expected_keys - actual_keys) + assert actual_keys == expected_keys, ( + f"{label} key drift: added={added!r} removed={removed!r}" + ) + assert actual == expected, f"{label} value mismatch" + + +def fixture_context(*, agent: AgentIdentity | None = None) -> HookContext: + """Build the shared deterministic invocation context for wire fixtures.""" + return HookContext( + thread_id="thread-1", + cwd=FIXED_CWD, + prompt_id=FIXED_PROMPT_ID, + approval_mode=ApprovalMode.MANUAL, + effort="high", + agent=agent, + ) + + +def representative_invocation(event: HookEvent) -> HookInvocation: + """Return a deterministic domain invocation for `event`. + + Args: + event: Lifecycle event to project onto the wire. + + Returns: + A domain invocation suitable for golden wire-input comparison. + """ + agent = AgentIdentity(id="agent-1", name="researcher") + domain = _representative_event(event, agent=agent) + identity = ( + agent if isinstance(domain, (SubagentStartEvent, SubagentStopEvent)) else None + ) + return HookInvocation(context=fixture_context(agent=identity), event=domain) + + +def agent_transcript_path_for(event: HookEvent) -> Path | None: + """Return the agent transcript path required by SubagentStop projection.""" + if event is HookEvent.SUBAGENT_STOP: + return FIXED_AGENT_TRANSCRIPT_PATH + return None + + +async def handler_result_from_exit( + *, + event: HookEvent, + exit_code: int, + stdout: str = "", + stderr: str = "", + handler_id: str = FIXED_HANDLER_ID, +) -> HandlerResult: + """Run a real command handler that reproduces one exit/stdout/stderr shape. + + Exit-code and output interpretation is owned by `run_command_handler`, so + these fixtures execute a real process rather than restating that mapping. + + Args: + event: Event the synthetic handler belongs to. + exit_code: Exit status the handler should return. + stdout: Text the handler should write to stdout. + stderr: Text the handler should write to stderr. + handler_id: Stable handler id recorded in diagnostics. + + Returns: + The handler result the reducer consumes. + """ + script = ( + "import sys;" + f"sys.stdin.buffer.read();" + f"sys.stdout.write({stdout!r});" + f"sys.stderr.write({stderr!r});" + f"sys.exit({exit_code})" + ) + handler = HookHandler( + id=handler_id, + event=event, + command="", + timeout=None, + status_message=None, + matcher=None, + matcher_text=None, + argv=(sys.executable, "-c", script), + ) + return await run_command_handler( + handler, + b"{}", + cwd=Path.cwd(), + default_timeout=_FIXTURE_HANDLER_TIMEOUT_SECONDS, + ) + + +def decision_snapshot(decision: HookDecision) -> JsonObject: + """Serialize a domain decision for fixture comparison.""" + dumped = HOOK_DECISION_ADAPTER.dump_python(decision, mode="json") + return cast("JsonObject", dumped) + + +def registry_policy_row(event: HookEvent) -> RegistryPolicyFixture: + """Build the policy-matrix row for `event` from the live capability registry.""" + spec = get_event_spec(event) + return { + "owner": spec.owner.value, + "matcher_field": spec.matcher_field, + "default_timeout_seconds": spec.default_timeout_seconds, + "exit_code_policy": spec.exit_code_policy.value, + "plain_output_policy": spec.plain_output_policy.value, + "aggregation_policy": spec.aggregation_policy.value, + } + + +def _representative_event( + event: HookEvent, + *, + agent: AgentIdentity, +) -> HookDomainEvent: + match event: + case HookEvent.SESSION_START: + return SessionStartEvent( + event=event, + cause=SessionStartCause.STARTUP, + model="provider:model", + ) + case HookEvent.USER_PROMPT_SUBMIT: + return UserPromptSubmitEvent(event=event, prompt="Review this change") + case HookEvent.SESSION_END: + return SessionEndEvent(event=event, cause=SessionEndCause.OTHER) + case HookEvent.PERMISSION_REQUEST: + return PermissionRequestEvent( + event=event, + call=ToolCallData(id="call-1", name="Bash", args={"command": "pwd"}), + ) + case HookEvent.NOTIFICATION: + return NotificationEvent( + event=event, + notification=DcodeNotification( + type=DcodeNotificationKind.PERMISSION_REQUIRED, + message="Approval required", + title="Permission", + ), + ) + case HookEvent.PRE_TOOL_USE: + return PreToolUseEvent( + event=event, + call=ToolCallData( + id="call-1", + name="Write", + args={"file_path": "notes.txt", "content": "hello"}, + ), + ) + case HookEvent.POST_TOOL_USE: + return PostToolUseEvent( + event=event, + call=ToolCallData(id="call-2", name="Bash", args={"command": "pwd"}), + result=ToolMessage( + content="/workspace", + tool_call_id="call-2", + name="Bash", + ), + duration_ms=12, + ) + case HookEvent.PRE_COMPACT: + return PreCompactEvent( + event=event, + trigger=CompactTrigger.MANUAL, + custom_instructions="Keep the plan", + ) + case HookEvent.STOP: + return StopEvent( + event=event, + continuation_count=0, + last_assistant_message="Done", + ) + case HookEvent.SUBAGENT_START: + return SubagentStartEvent(event=event, agent=agent) + case HookEvent.SUBAGENT_STOP: + return SubagentStopEvent( + event=event, + agent=agent, + continuation_count=0, + last_assistant_message="Found it", + ) + case _: + msg = f"Unsupported hook event: {event}" + raise ValueError(msg) diff --git a/libs/code/tests/unit_tests/tui/widgets/test_status.py b/libs/code/tests/unit_tests/tui/widgets/test_status.py index 5abe12a2620..7c817eb9163 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_status.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_status.py @@ -523,6 +523,37 @@ async def test_setting_message_shows_then_clearing_hides(self) -> None: await pilot.pause() assert msg.display is False + async def test_hook_and_agent_status_do_not_clobber(self) -> None: + """Hook and agent writers acquire/release the shared slot without clobber.""" + async with StatusBarApp().run_test() as pilot: + bar = pilot.app.query_one("#status-bar", StatusBar) + msg = pilot.app.query_one("#status-message", Static) + + bar.set_status_message("Loading thread", source="agent") + await pilot.pause() + assert str(msg.render()) == "Loading thread" + + bar.set_status_message("Running [lint] checks", source="hooks") + await pilot.pause() + assert str(msg.render()) == "Running [lint] checks" + + bar.set_status_message("Still loading", source="agent") + await pilot.pause() + assert str(msg.render()) == "Running [lint] checks" + + bar.set_status_message("", source="hooks") + await pilot.pause() + assert str(msg.render()) == "Still loading" + + async def test_status_message_renders_markup_literally(self) -> None: + """Configured status text must not be interpreted as Rich markup.""" + async with StatusBarApp().run_test() as pilot: + bar = pilot.app.query_one("#status-bar", StatusBar) + msg = pilot.app.query_one("#status-message", Static) + bar.set_status_message("Running [bold]hook[/bold]", source="hooks") + await pilot.pause() + assert str(msg.render()) == "Running [bold]hook[/bold]" + async def test_busy_shows_slot_and_clearing_hides(self) -> None: """A busy indicator reveals the slot; clearing busy (no message) hides it.""" async with StatusBarApp().run_test() as pilot: From 16dca56ed8a508a06bb44b6195cf9fb9ae333f9e Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Mon, 27 Jul 2026 16:50:26 -0700 Subject: [PATCH 2/6] test(code): drop unrelated wire-fixture subsystem from runtime-feedback PR --- .../unit_tests/hooks/fixtures/__init__.py | 1 - .../unit_tests/hooks/fixtures/wire/README.md | 29 - .../hooks/fixtures/wire/__init__.py | 5 - .../fixtures/wire/inputs/Notification.json | 14 - .../wire/inputs/PermissionRequest.json | 16 - .../fixtures/wire/inputs/PostToolUse.json | 28 - .../fixtures/wire/inputs/PreCompact.json | 13 - .../fixtures/wire/inputs/PreToolUse.json | 17 - .../fixtures/wire/inputs/SessionEnd.json | 12 - .../fixtures/wire/inputs/SessionStart.json | 13 - .../hooks/fixtures/wire/inputs/Stop.json | 15 - .../fixtures/wire/inputs/SubagentStart.json | 13 - .../fixtures/wire/inputs/SubagentStop.json | 18 - .../wire/inputs/UserPromptSubmit.json | 12 - .../wire/outputs/reduction_cases.json | 540 ------------------ .../fixtures/wire/registry_policies.json | 90 --- .../unit_tests/hooks/test_wire_fixtures.py | 146 ----- .../unit_tests/hooks/wire_fixture_helpers.py | 358 ------------ 18 files changed, 1340 deletions(-) delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/__init__.py delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/README.md delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json delete mode 100644 libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json delete mode 100644 libs/code/tests/unit_tests/hooks/test_wire_fixtures.py delete mode 100644 libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py diff --git a/libs/code/tests/unit_tests/hooks/fixtures/__init__.py b/libs/code/tests/unit_tests/hooks/fixtures/__init__.py deleted file mode 100644 index 7272e9c2857..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/__init__.py +++ /dev/null @@ -1 +0,0 @@ -"""Test fixture packages for Hooks unit tests.""" diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md b/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md deleted file mode 100644 index dfd86f06921..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/README.md +++ /dev/null @@ -1,29 +0,0 @@ -# Hooks wire-contract fixtures - -These JSON files pin the **external** Hooks v2 wire contract — the stdin payload -shape handlers receive, the registry policy matrix that drives matcher selection -and exit-code / plain-output behavior, and the domain decisions produced when -representative handler exits are reduced. - -## When a test fails - -A failing diff means one of: - -1. **Deliberate contract change** — update the matching fixture in the same PR - and call out the wire/policy change in the PR description. -2. **Accidental regression** — restore the previous shape; do not loosen the - assertion. - -Do not edit these files to silence a failure without understanding whether the -external contract changed. - -## Layout - -| Path | Pins | -| --- | --- | -| `inputs/.json` | Serialized wire input for one lifecycle event (external field names) | -| `registry_policies.json` | Per-event owner, matcher field, timeouts, and exit/plain/aggregation policies | -| `outputs/reduction_cases.json` | Handler exit scenarios → reduced domain decision snapshots | - -Volatile values (`prompt_id`, path fields) are normalized to placeholders in the -test helper before comparison so fixtures stay stable across environments. diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py b/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py deleted file mode 100644 index 4465a9af76b..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/__init__.py +++ /dev/null @@ -1,5 +0,0 @@ -"""Committed Hooks v2 wire-contract fixtures. - -See `README.md` in this directory: failing diffs are either deliberate contract -changes (update the fixture and document them) or accidental regressions. -""" diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json deleted file mode 100644 index f99f31a1a7e..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Notification.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "Notification", - "message": "Approval required", - "notification_type": "permission_prompt", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "title": "Permission", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json deleted file mode 100644 index 1b902a07d93..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PermissionRequest.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "PermissionRequest", - "permission_mode": "default", - "permission_suggestions": [], - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "tool_input": { - "command": "pwd" - }, - "tool_name": "Bash", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json deleted file mode 100644 index acffbc0fa3e..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PostToolUse.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "cwd": "/workspace", - "duration_ms": 12, - "effort": { - "level": "high" - }, - "hook_event_name": "PostToolUse", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "tool_input": { - "command": "pwd" - }, - "tool_name": "Bash", - "tool_response": { - "additional_kwargs": {}, - "artifact": null, - "content": "/workspace", - "id": null, - "name": "Bash", - "response_metadata": {}, - "status": "success", - "tool_call_id": "call-2", - "type": "tool" - }, - "tool_use_id": "call-2", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json deleted file mode 100644 index bea1b536900..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreCompact.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "custom_instructions": "Keep the plan", - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "PreCompact", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "transcript_path": "/tmp/thread.jsonl", - "trigger": "manual" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json deleted file mode 100644 index cf60a4e6fca..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/PreToolUse.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "PreToolUse", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "tool_input": { - "content": "hello", - "file_path": "notes.txt" - }, - "tool_name": "Write", - "tool_use_id": "call-1", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json deleted file mode 100644 index d84183ed45b..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionEnd.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "SessionEnd", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "reason": "other", - "session_id": "thread-1", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json deleted file mode 100644 index 184fcacb7bb..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SessionStart.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "SessionStart", - "model": "provider:model", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "source": "startup", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json deleted file mode 100644 index af323e30012..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/Stop.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "background_tasks": [], - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "Stop", - "last_assistant_message": "Done", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_crons": [], - "session_id": "thread-1", - "stop_hook_active": false, - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json deleted file mode 100644 index 5114104cf74..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStart.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "agent_id": "agent-1", - "agent_type": "researcher", - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "SubagentStart", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json deleted file mode 100644 index fc801ff5073..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/SubagentStop.json +++ /dev/null @@ -1,18 +0,0 @@ -{ - "agent_id": "agent-1", - "agent_transcript_path": "/tmp/agent.jsonl", - "agent_type": "researcher", - "background_tasks": [], - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "SubagentStop", - "last_assistant_message": "Found it", - "permission_mode": "default", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_crons": [], - "session_id": "thread-1", - "stop_hook_active": false, - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json deleted file mode 100644 index 86f8febab1c..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/inputs/UserPromptSubmit.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "cwd": "/workspace", - "effort": { - "level": "high" - }, - "hook_event_name": "UserPromptSubmit", - "permission_mode": "default", - "prompt": "Review this change", - "prompt_id": "00000000-0000-4000-8000-000000000001", - "session_id": "thread-1", - "transcript_path": "/tmp/thread.jsonl" -} diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json deleted file mode 100644 index 150dbd3789c..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/outputs/reduction_cases.json +++ /dev/null @@ -1,540 +0,0 @@ -[ - { - "event": "UserPromptSubmit", - "exit_code": 0, - "expected": { - "context": [ - "Model context from JSON" - ], - "continue_processing": true, - "diagnostics": [], - "event": "UserPromptSubmit", - "stop_reason": null, - "suppress_original_prompt": false, - "terminal_sequences": [], - "user_notices": [ - "Visible notice" - ] - }, - "id": "UserPromptSubmit_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"Visible notice\", \"hookSpecificOutput\": {\"hookEventName\": \"UserPromptSubmit\", \"additionalContext\": \"Model context from JSON\"}}" - }, - { - "event": "UserPromptSubmit", - "exit_code": 2, - "expected": { - "context": [], - "continue_processing": false, - "diagnostics": [], - "event": "UserPromptSubmit", - "stop_reason": "Blocked by policy", - "suppress_original_prompt": false, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "UserPromptSubmit_exit2", - "stderr": "Blocked by policy", - "stdout": "" - }, - { - "event": "UserPromptSubmit", - "exit_code": 1, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 1", - "severity": "warning" - } - ], - "event": "UserPromptSubmit", - "stop_reason": null, - "suppress_original_prompt": false, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "UserPromptSubmit_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "UserPromptSubmit", - "exit_code": 0, - "expected": { - "context": [ - "plain context line" - ], - "continue_processing": true, - "diagnostics": [], - "event": "UserPromptSubmit", - "stop_reason": null, - "suppress_original_prompt": false, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "UserPromptSubmit_plain", - "stderr": "", - "stdout": "plain context line" - }, - { - "event": "PreToolUse", - "exit_code": 0, - "expected": { - "context": [ - "Pre-tool context" - ], - "continue_processing": true, - "diagnostics": [], - "event": "PreToolUse", - "permission": { - "behavior": "deny", - "interrupt": false, - "reason": "Protected path" - }, - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [ - "Pre-tool notice" - ] - }, - "id": "PreToolUse_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"Pre-tool notice\", \"hookSpecificOutput\": {\"hookEventName\": \"PreToolUse\", \"permissionDecision\": \"deny\", \"permissionDecisionReason\": \"Protected path\", \"additionalContext\": \"Pre-tool context\"}}" - }, - { - "event": "PreToolUse", - "exit_code": 2, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [], - "event": "PreToolUse", - "permission": { - "behavior": "deny", - "interrupt": false, - "reason": "Denied by exit 2" - }, - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PreToolUse_exit2", - "stderr": "Denied by exit 2", - "stdout": "" - }, - { - "event": "PreToolUse", - "exit_code": 3, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 3", - "severity": "warning" - } - ], - "event": "PreToolUse", - "permission": { - "behavior": "none", - "interrupt": false, - "reason": null - }, - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PreToolUse_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "PreToolUse", - "exit_code": 0, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "malformed_json", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook output is not valid JSON", - "severity": "warning" - } - ], - "event": "PreToolUse", - "permission": { - "behavior": "none", - "interrupt": false, - "reason": null - }, - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PreToolUse_plain", - "stderr": "", - "stdout": "not-json" - }, - { - "event": "PostToolUse", - "exit_code": 0, - "expected": { - "context": [ - "Post-tool context" - ], - "continue_processing": true, - "diagnostics": [], - "event": "PostToolUse", - "feedback": [], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [ - "Post-tool notice" - ] - }, - "id": "PostToolUse_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"Post-tool notice\", \"hookSpecificOutput\": {\"hookEventName\": \"PostToolUse\", \"additionalContext\": \"Post-tool context\"}}" - }, - { - "event": "PostToolUse", - "exit_code": 2, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [], - "event": "PostToolUse", - "feedback": [ - "Feedback from exit 2" - ], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PostToolUse_exit2", - "stderr": "Feedback from exit 2", - "stdout": "" - }, - { - "event": "PostToolUse", - "exit_code": 1, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 1", - "severity": "warning" - } - ], - "event": "PostToolUse", - "feedback": [], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PostToolUse_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "PostToolUse", - "exit_code": 0, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "malformed_json", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook output is not valid JSON", - "severity": "warning" - } - ], - "event": "PostToolUse", - "feedback": [], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "PostToolUse_plain", - "stderr": "", - "stdout": "ignored plain" - }, - { - "event": "SessionStart", - "exit_code": 0, - "expected": { - "context": [ - "Session context" - ], - "continue_processing": true, - "diagnostics": [], - "event": "SessionStart", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [ - "Session notice" - ] - }, - "id": "SessionStart_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"Session notice\", \"hookSpecificOutput\": {\"hookEventName\": \"SessionStart\", \"additionalContext\": \"Session context\"}}" - }, - { - "event": "SessionStart", - "exit_code": 2, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "unsupported_block", - "field": "decision", - "handler_id": "fixture:0:0", - "message": "Block/exit 2 is not supported for SessionStart: Unsupported block", - "severity": "warning" - } - ], - "event": "SessionStart", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionStart_exit2", - "stderr": "Unsupported block", - "stdout": "" - }, - { - "event": "SessionStart", - "exit_code": 1, - "expected": { - "context": [], - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 1", - "severity": "warning" - } - ], - "event": "SessionStart", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionStart_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "SessionStart", - "exit_code": 0, - "expected": { - "context": [ - "startup plain context" - ], - "continue_processing": true, - "diagnostics": [], - "event": "SessionStart", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionStart_plain", - "stderr": "", - "stdout": "startup plain context" - }, - { - "event": "Stop", - "exit_code": 0, - "expected": { - "continue_loop": true, - "continue_processing": true, - "diagnostics": [], - "event": "Stop", - "feedback": [ - "Continue with this" - ], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [ - "Stop notice" - ] - }, - "id": "Stop_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"Stop notice\", \"hookSpecificOutput\": {\"hookEventName\": \"Stop\", \"additionalContext\": \"Continue with this\"}}" - }, - { - "event": "Stop", - "exit_code": 2, - "expected": { - "continue_loop": true, - "continue_processing": true, - "diagnostics": [], - "event": "Stop", - "feedback": [ - "Please continue" - ], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "Stop_exit2", - "stderr": "Please continue", - "stdout": "" - }, - { - "event": "Stop", - "exit_code": 1, - "expected": { - "continue_loop": false, - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 1", - "severity": "warning" - } - ], - "event": "Stop", - "feedback": [], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "Stop_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "Stop", - "exit_code": 0, - "expected": { - "continue_loop": false, - "continue_processing": true, - "diagnostics": [ - { - "code": "malformed_json", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook output is not valid JSON", - "severity": "warning" - } - ], - "event": "Stop", - "feedback": [], - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "Stop_plain", - "stderr": "", - "stdout": "stop plain ignored" - }, - { - "event": "SessionEnd", - "exit_code": 0, - "expected": { - "continue_processing": true, - "diagnostics": [], - "event": "SessionEnd", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [ - "End notice" - ] - }, - "id": "SessionEnd_exit0_json", - "stderr": "", - "stdout": "{\"continue\": true, \"systemMessage\": \"End notice\"}" - }, - { - "event": "SessionEnd", - "exit_code": 2, - "expected": { - "continue_processing": true, - "diagnostics": [ - { - "code": "unsupported_block", - "field": "decision", - "handler_id": "fixture:0:0", - "message": "Block/exit 2 is not supported for SessionEnd: ignored block", - "severity": "warning" - } - ], - "event": "SessionEnd", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionEnd_exit2", - "stderr": "ignored block", - "stdout": "" - }, - { - "event": "SessionEnd", - "exit_code": 1, - "expected": { - "continue_processing": true, - "diagnostics": [ - { - "code": "nonzero_exit", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook exited with status 1", - "severity": "warning" - } - ], - "event": "SessionEnd", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionEnd_nonzero", - "stderr": "", - "stdout": "" - }, - { - "event": "SessionEnd", - "exit_code": 0, - "expected": { - "continue_processing": true, - "diagnostics": [ - { - "code": "malformed_json", - "field": null, - "handler_id": "fixture:0:0", - "message": "Hook output is not valid JSON", - "severity": "warning" - } - ], - "event": "SessionEnd", - "stop_reason": null, - "terminal_sequences": [], - "user_notices": [] - }, - "id": "SessionEnd_plain", - "stderr": "", - "stdout": "end plain ignored" - } -] diff --git a/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json b/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json deleted file mode 100644 index 8d37a504fd6..00000000000 --- a/libs/code/tests/unit_tests/hooks/fixtures/wire/registry_policies.json +++ /dev/null @@ -1,90 +0,0 @@ -{ - "Notification": { - "aggregation_policy": "side_effect", - "default_timeout_seconds": 600.0, - "exit_code_policy": "diagnose", - "matcher_field": "notification_type", - "owner": "client", - "plain_output_policy": "ignore" - }, - "PermissionRequest": { - "aggregation_policy": "permission", - "default_timeout_seconds": 600.0, - "exit_code_policy": "deny", - "matcher_field": "tool_name", - "owner": "client", - "plain_output_policy": "ignore" - }, - "PostToolUse": { - "aggregation_policy": "feedback_and_context", - "default_timeout_seconds": 600.0, - "exit_code_policy": "feedback", - "matcher_field": "tool_name", - "owner": "server", - "plain_output_policy": "ignore" - }, - "PreCompact": { - "aggregation_policy": "side_effect", - "default_timeout_seconds": 600.0, - "exit_code_policy": "block", - "matcher_field": "trigger", - "owner": "server", - "plain_output_policy": "ignore" - }, - "PreToolUse": { - "aggregation_policy": "permission", - "default_timeout_seconds": 600.0, - "exit_code_policy": "deny", - "matcher_field": "tool_name", - "owner": "server", - "plain_output_policy": "ignore" - }, - "SessionEnd": { - "aggregation_policy": "side_effect", - "default_timeout_seconds": 600.0, - "exit_code_policy": "diagnose", - "matcher_field": "cause", - "owner": "client", - "plain_output_policy": "ignore" - }, - "SessionStart": { - "aggregation_policy": "context", - "default_timeout_seconds": 600.0, - "exit_code_policy": "diagnose", - "matcher_field": "cause", - "owner": "client", - "plain_output_policy": "context" - }, - "Stop": { - "aggregation_policy": "stop_loop", - "default_timeout_seconds": 600.0, - "exit_code_policy": "continue_loop", - "matcher_field": null, - "owner": "server", - "plain_output_policy": "ignore" - }, - "SubagentStart": { - "aggregation_policy": "context", - "default_timeout_seconds": 600.0, - "exit_code_policy": "diagnose", - "matcher_field": "agent_name", - "owner": "server", - "plain_output_policy": "ignore" - }, - "SubagentStop": { - "aggregation_policy": "context", - "default_timeout_seconds": 600.0, - "exit_code_policy": "context", - "matcher_field": "agent_name", - "owner": "server", - "plain_output_policy": "ignore" - }, - "UserPromptSubmit": { - "aggregation_policy": "context", - "default_timeout_seconds": 30.0, - "exit_code_policy": "block", - "matcher_field": null, - "owner": "client", - "plain_output_policy": "context" - } -} diff --git a/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py b/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py deleted file mode 100644 index 16a8a97d580..00000000000 --- a/libs/code/tests/unit_tests/hooks/test_wire_fixtures.py +++ /dev/null @@ -1,146 +0,0 @@ -"""Differential golden fixtures for the Hooks v2 external wire contract. - -Committed JSON under `fixtures/wire/` pins stdin payloads, registry policy -rows, and handler-exit → domain-decision interpretation. Exhaustiveness is -enforced by iterating `HookEvent` / the capability registry — a new event -without a fixture fails the suite. -""" - -from __future__ import annotations - -import json -from operator import itemgetter -from typing import TYPE_CHECKING, cast - -import pytest - -from deepagents_code.hooks.models.adapters import HOOK_WIRE_INPUT_ADAPTER -from deepagents_code.hooks.models.domain import HookEvent -from deepagents_code.hooks.projection import project_hook_input -from deepagents_code.hooks.reducer import reduce_hook_results -from unit_tests.hooks.wire_fixture_helpers import ( - FIXED_TRANSCRIPT_PATH, - INPUTS_DIR, - REDUCTION_CASES_PATH, - ReductionCaseFixture, - agent_transcript_path_for, - assert_exact_mapping, - decision_snapshot, - handler_result_from_exit, - load_reduction_cases, - load_registry_policies, - load_wire_input_fixture, - normalize_wire_payload, - registry_policy_row, - representative_invocation, -) - -if TYPE_CHECKING: - from deepagents_code.json_types import JsonObject - - -def test_wire_input_fixtures_cover_every_registry_event() -> None: - fixture_events = {path.stem for path in INPUTS_DIR.glob("*.json")} - registry_events = {event.value for event in HookEvent} - assert fixture_events == registry_events, ( - "Wire input fixtures must cover every HookEvent exactly: " - f"missing={sorted(registry_events - fixture_events)!r} " - f"extra={sorted(fixture_events - registry_events)!r}" - ) - - -@pytest.mark.parametrize("event", list(HookEvent), ids=lambda event: event.value) -def test_projected_wire_input_matches_fixture(event: HookEvent) -> None: - invocation = representative_invocation(event) - projected = HOOK_WIRE_INPUT_ADAPTER.dump_python( - project_hook_input( - invocation, - transcript_path=FIXED_TRANSCRIPT_PATH, - agent_transcript_path=agent_transcript_path_for(event), - ), - mode="json", - by_alias=True, - exclude_none=True, - ) - actual = normalize_wire_payload(cast("JsonObject", projected)) - expected = normalize_wire_payload(load_wire_input_fixture(event)) - assert_exact_mapping(actual, expected, label=f"{event.value} wire input") - - -def test_registry_policy_fixture_covers_every_event() -> None: - policies = load_registry_policies() - registry_events = {event.value for event in HookEvent} - assert set(policies) == registry_events, ( - "registry_policies.json must cover every HookEvent exactly: " - f"missing={sorted(registry_events - set(policies))!r} " - f"extra={sorted(set(policies) - registry_events)!r}" - ) - - -@pytest.mark.parametrize("event", list(HookEvent), ids=lambda event: event.value) -def test_registry_policy_matches_fixture(event: HookEvent) -> None: - policies = load_registry_policies() - actual = cast("JsonObject", dict(registry_policy_row(event))) - expected = cast("JsonObject", dict(policies[event.value])) - assert_exact_mapping(actual, expected, label=f"{event.value} registry policy") - - -def test_reduction_cases_fixture_is_nonempty() -> None: - cases = load_reduction_cases() - assert cases, f"Expected reduction cases in {REDUCTION_CASES_PATH}" - ids = [case["id"] for case in cases] - assert len(ids) == len(set(ids)), f"Duplicate reduction case ids: {ids!r}" - - -@pytest.mark.parametrize( - "case", - load_reduction_cases(), - ids=itemgetter("id"), -) -async def test_handler_output_reduction_matches_fixture( - case: ReductionCaseFixture, -) -> None: - event = HookEvent(case["event"]) - invocation = representative_invocation(event) - result = await handler_result_from_exit( - event=event, - exit_code=case["exit_code"], - stdout=case["stdout"], - stderr=case["stderr"], - ) - decision = reduce_hook_results(invocation, [result]) - actual = decision_snapshot(decision) - assert_exact_mapping( - actual, - case["expected"], - label=f"{case['id']} reduced decision", - ) - - -def test_reduction_cases_cover_required_exit_shapes() -> None: - """Pin the verification matrix: exit 0 JSON, exit 2, other nonzero, plain.""" - cases = load_reduction_cases() - by_event: dict[str, set[str]] = {} - for case in cases: - shapes = by_event.setdefault(case["event"], set()) - if case["exit_code"] == 0 and case["stdout"].lstrip().startswith("{"): - shapes.add("exit0_json") - elif case["exit_code"] == 2: - shapes.add("exit2") - elif case["exit_code"] != 0: - shapes.add("nonzero") - elif case["exit_code"] == 0: - shapes.add("plain") - required = {"exit0_json", "exit2", "nonzero", "plain"} - assert by_event, "No reduction cases loaded" - for event, shapes in by_event.items(): - assert shapes == required, ( - f"{event} reduction cases missing shapes: " - f"have={sorted(shapes)!r} need={sorted(required)!r}" - ) - - -def test_wire_input_fixture_files_are_objects() -> None: - for path in sorted(INPUTS_DIR.glob("*.json")): - raw: object = json.loads(path.read_text(encoding="utf-8")) - assert isinstance(raw, dict), f"{path.name} must be a JSON object" diff --git a/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py b/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py deleted file mode 100644 index b0c27a04553..00000000000 --- a/libs/code/tests/unit_tests/hooks/wire_fixture_helpers.py +++ /dev/null @@ -1,358 +0,0 @@ -"""Typed helpers for Hooks v2 differential wire fixtures.""" - -from __future__ import annotations - -import copy -import json -import sys -from pathlib import Path -from typing import TYPE_CHECKING, Final, TypedDict, cast -from uuid import UUID - -from langchain_core.messages import ToolMessage - -from deepagents_code.approval_mode import ApprovalMode -from deepagents_code.hooks.capabilities import get_event_spec -from deepagents_code.hooks.models.adapters import HOOK_DECISION_ADAPTER -from deepagents_code.hooks.models.domain import ( - AgentIdentity, - CompactTrigger, - DcodeNotification, - DcodeNotificationKind, - HookContext, - HookDiagnostic, - HookEvent, - HookInvocation, - NotificationEvent, - PermissionRequestEvent, - PostToolUseEvent, - PreCompactEvent, - PreToolUseEvent, - SessionEndCause, - SessionEndEvent, - SessionStartCause, - SessionStartEvent, - StopEvent, - SubagentStartEvent, - SubagentStopEvent, - ToolCallData, - UserPromptSubmitEvent, -) -from deepagents_code.hooks.runner import HandlerResult, run_command_handler -from deepagents_code.hooks.snapshot import HookHandler - -if TYPE_CHECKING: - from deepagents_code.hooks.models.domain import HookDecision, HookDomainEvent - from deepagents_code.json_types import JsonObject - -FIXTURES_DIR: Final[Path] = Path(__file__).resolve().parent / "fixtures" / "wire" -INPUTS_DIR: Final[Path] = FIXTURES_DIR / "inputs" -OUTPUTS_DIR: Final[Path] = FIXTURES_DIR / "outputs" -REGISTRY_POLICIES_PATH: Final[Path] = FIXTURES_DIR / "registry_policies.json" -REDUCTION_CASES_PATH: Final[Path] = OUTPUTS_DIR / "reduction_cases.json" - -FIXED_PROMPT_ID: Final[UUID] = UUID("00000000-0000-4000-8000-000000000001") -FIXED_CWD: Final[Path] = Path("/workspace") -FIXED_TRANSCRIPT_PATH: Final[Path] = Path("/tmp/thread.jsonl") -FIXED_AGENT_TRANSCRIPT_PATH: Final[Path] = Path("/tmp/agent.jsonl") -FIXED_HANDLER_ID: Final[str] = "fixture:0:0" - -# Keys whose values are inherently environment- or generation-dependent. -# Both the projected payload and the fixture pass through normalize_wire_payload -# so committed fixtures may store concrete sample values while comparison uses -# stable placeholders. -_VOLATILE_WIRE_KEYS: Final[dict[str, str]] = { - "prompt_id": "", - "cwd": "", - "transcript_path": "", - "agent_transcript_path": "", -} - -_FIXTURE_HANDLER_TIMEOUT_SECONDS: Final[float] = 20.0 - - -class RegistryPolicyFixture(TypedDict): - """Pinned capability-matrix row for one hook event.""" - - owner: str - matcher_field: str | None - default_timeout_seconds: float - exit_code_policy: str - plain_output_policy: str - aggregation_policy: str - - -class ReductionCaseFixture(TypedDict): - """One handler-exit scenario and its reduced domain decision snapshot.""" - - id: str - event: str - exit_code: int - stdout: str - stderr: str - expected: JsonObject - - -def load_json_object(path: Path) -> JsonObject: - """Load a JSON object fixture from disk. - - Args: - path: Absolute or relative path to a JSON object file. - - Returns: - The parsed JSON object. - - Raises: - ValueError: If the file does not contain a JSON object. - """ - raw: object = json.loads(path.read_text(encoding="utf-8")) - if not isinstance(raw, dict): - msg = f"Fixture must be a JSON object: {path}" - raise TypeError(msg) - return cast("JsonObject", raw) - - -def load_registry_policies() -> dict[str, RegistryPolicyFixture]: - """Load the pinned registry policy matrix.""" - raw = load_json_object(REGISTRY_POLICIES_PATH) - return cast("dict[str, RegistryPolicyFixture]", raw) - - -def load_reduction_cases() -> list[ReductionCaseFixture]: - """Load handler-output reduction cases.""" - raw: object = json.loads(REDUCTION_CASES_PATH.read_text(encoding="utf-8")) - if not isinstance(raw, list): - msg = f"Reduction cases fixture must be a JSON array: {REDUCTION_CASES_PATH}" - raise TypeError(msg) - return cast("list[ReductionCaseFixture]", raw) - - -def load_wire_input_fixture(event: HookEvent) -> JsonObject: - """Load the committed wire-input fixture for `event`.""" - return load_json_object(INPUTS_DIR / f"{event.value}.json") - - -def normalize_wire_payload(payload: JsonObject) -> JsonObject: - """Stabilize non-deterministic wire fields for exact fixture comparison. - - Rewrites known volatile keys (`prompt_id`, `cwd`, `transcript_path`, - `agent_transcript_path`) to documented placeholders. All other keys and - nested values are left unchanged so added, removed, or renamed fields fail - the exact comparison. - - Args: - payload: Serialized hook wire input (external field names). - - Returns: - A deep copy with volatile keys replaced by placeholders. - """ - normalized = cast("JsonObject", copy.deepcopy(payload)) - for key, placeholder in _VOLATILE_WIRE_KEYS.items(): - if key in normalized: - normalized[key] = placeholder - return normalized - - -def assert_exact_mapping( - actual: JsonObject, - expected: JsonObject, - *, - label: str, -) -> None: - """Assert two JSON objects match with exact key sets. - - Args: - actual: Observed mapping. - expected: Fixture mapping. - label: Context included in assertion messages. - - Raises: - AssertionError: On key drift or value mismatch. - """ - actual_keys = set(actual) - expected_keys = set(expected) - added = sorted(actual_keys - expected_keys) - removed = sorted(expected_keys - actual_keys) - assert actual_keys == expected_keys, ( - f"{label} key drift: added={added!r} removed={removed!r}" - ) - assert actual == expected, f"{label} value mismatch" - - -def fixture_context(*, agent: AgentIdentity | None = None) -> HookContext: - """Build the shared deterministic invocation context for wire fixtures.""" - return HookContext( - thread_id="thread-1", - cwd=FIXED_CWD, - prompt_id=FIXED_PROMPT_ID, - approval_mode=ApprovalMode.MANUAL, - effort="high", - agent=agent, - ) - - -def representative_invocation(event: HookEvent) -> HookInvocation: - """Return a deterministic domain invocation for `event`. - - Args: - event: Lifecycle event to project onto the wire. - - Returns: - A domain invocation suitable for golden wire-input comparison. - """ - agent = AgentIdentity(id="agent-1", name="researcher") - domain = _representative_event(event, agent=agent) - identity = ( - agent if isinstance(domain, (SubagentStartEvent, SubagentStopEvent)) else None - ) - return HookInvocation(context=fixture_context(agent=identity), event=domain) - - -def agent_transcript_path_for(event: HookEvent) -> Path | None: - """Return the agent transcript path required by SubagentStop projection.""" - if event is HookEvent.SUBAGENT_STOP: - return FIXED_AGENT_TRANSCRIPT_PATH - return None - - -async def handler_result_from_exit( - *, - event: HookEvent, - exit_code: int, - stdout: str = "", - stderr: str = "", - handler_id: str = FIXED_HANDLER_ID, -) -> HandlerResult: - """Run a real command handler that reproduces one exit/stdout/stderr shape. - - Exit-code and output interpretation is owned by `run_command_handler`, so - these fixtures execute a real process rather than restating that mapping. - - Args: - event: Event the synthetic handler belongs to. - exit_code: Exit status the handler should return. - stdout: Text the handler should write to stdout. - stderr: Text the handler should write to stderr. - handler_id: Stable handler id recorded in diagnostics. - - Returns: - The handler result the reducer consumes. - """ - script = ( - "import sys;" - f"sys.stdin.buffer.read();" - f"sys.stdout.write({stdout!r});" - f"sys.stderr.write({stderr!r});" - f"sys.exit({exit_code})" - ) - handler = HookHandler( - id=handler_id, - event=event, - command="", - timeout=None, - status_message=None, - matcher=None, - matcher_text=None, - argv=(sys.executable, "-c", script), - ) - return await run_command_handler( - handler, - b"{}", - cwd=Path.cwd(), - default_timeout=_FIXTURE_HANDLER_TIMEOUT_SECONDS, - ) - - -def decision_snapshot(decision: HookDecision) -> JsonObject: - """Serialize a domain decision for fixture comparison.""" - dumped = HOOK_DECISION_ADAPTER.dump_python(decision, mode="json") - return cast("JsonObject", dumped) - - -def registry_policy_row(event: HookEvent) -> RegistryPolicyFixture: - """Build the policy-matrix row for `event` from the live capability registry.""" - spec = get_event_spec(event) - return { - "owner": spec.owner.value, - "matcher_field": spec.matcher_field, - "default_timeout_seconds": spec.default_timeout_seconds, - "exit_code_policy": spec.exit_code_policy.value, - "plain_output_policy": spec.plain_output_policy.value, - "aggregation_policy": spec.aggregation_policy.value, - } - - -def _representative_event( - event: HookEvent, - *, - agent: AgentIdentity, -) -> HookDomainEvent: - match event: - case HookEvent.SESSION_START: - return SessionStartEvent( - event=event, - cause=SessionStartCause.STARTUP, - model="provider:model", - ) - case HookEvent.USER_PROMPT_SUBMIT: - return UserPromptSubmitEvent(event=event, prompt="Review this change") - case HookEvent.SESSION_END: - return SessionEndEvent(event=event, cause=SessionEndCause.OTHER) - case HookEvent.PERMISSION_REQUEST: - return PermissionRequestEvent( - event=event, - call=ToolCallData(id="call-1", name="Bash", args={"command": "pwd"}), - ) - case HookEvent.NOTIFICATION: - return NotificationEvent( - event=event, - notification=DcodeNotification( - type=DcodeNotificationKind.PERMISSION_REQUIRED, - message="Approval required", - title="Permission", - ), - ) - case HookEvent.PRE_TOOL_USE: - return PreToolUseEvent( - event=event, - call=ToolCallData( - id="call-1", - name="Write", - args={"file_path": "notes.txt", "content": "hello"}, - ), - ) - case HookEvent.POST_TOOL_USE: - return PostToolUseEvent( - event=event, - call=ToolCallData(id="call-2", name="Bash", args={"command": "pwd"}), - result=ToolMessage( - content="/workspace", - tool_call_id="call-2", - name="Bash", - ), - duration_ms=12, - ) - case HookEvent.PRE_COMPACT: - return PreCompactEvent( - event=event, - trigger=CompactTrigger.MANUAL, - custom_instructions="Keep the plan", - ) - case HookEvent.STOP: - return StopEvent( - event=event, - continuation_count=0, - last_assistant_message="Done", - ) - case HookEvent.SUBAGENT_START: - return SubagentStartEvent(event=event, agent=agent) - case HookEvent.SUBAGENT_STOP: - return SubagentStopEvent( - event=event, - agent=agent, - continuation_count=0, - last_assistant_message="Found it", - ) - case _: - msg = f"Unsupported hook event: {event}" - raise ValueError(msg) From 6ccadf418172efa1fe20061123d8a725d976a043 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Mon, 27 Jul 2026 17:13:52 -0700 Subject: [PATCH 3/6] test(code): trim runtime-feedback coverage --- .../unit_tests/hooks/test_client_lifecycle.py | 3 +- .../tests/unit_tests/hooks/test_engine.py | 145 +------------- .../tests/unit_tests/hooks/test_feedback.py | 187 ++---------------- .../unit_tests/hooks/test_server_lifecycle.py | 45 +---- .../unit_tests/tui/widgets/test_status.py | 19 +- 5 files changed, 28 insertions(+), 371 deletions(-) diff --git a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py index 923522c3bfb..0a23d41e771 100644 --- a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py @@ -78,7 +78,8 @@ async def test_common_effects_context_and_live_hook_fields( severity="warning", message="diagnostic", ) - ], + ] + * 2, ) ] ), diff --git a/libs/code/tests/unit_tests/hooks/test_engine.py b/libs/code/tests/unit_tests/hooks/test_engine.py index 423bc8ee82f..2f282d771c3 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -2,7 +2,6 @@ from __future__ import annotations -import asyncio import json import subprocess import sys @@ -1645,50 +1644,6 @@ async def test_engine_reports_configured_handler_status(tmp_path: Path) -> None: } ) ) - invocation = _invocation( - tmp_path, - SessionStartEvent( - event=HookEvent.SESSION_START, - cause=SessionStartCause.STARTUP, - ), - ) - progress: list[HookProgress] = [] - - await HookEngine(snapshot).run( - invocation, - transcript_path=_transcript_path(tmp_path), - progress=progress.append, - ) - - assert [update.active for update in progress] == [True, False] - assert {update.message for update in progress} == {"Loading project context"} - - -async def test_engine_default_progress_uses_charset_glyphs( - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - from deepagents_code.config import get_glyphs, reset_glyphs_cache - - monkeypatch.setenv("UI_CHARSET_MODE", "ascii") - reset_glyphs_cache() - snapshot = HooksSnapshot.from_config( - _config( - { - "SessionStart": [ - { - "hooks": [ - { - "type": "command", - "command": "unused", - "argv": [sys.executable, "-c", "pass"], - } - ] - } - ] - } - ) - ) progress: list[HookProgress] = [] await HookEngine(snapshot).run( @@ -1703,102 +1658,10 @@ async def test_engine_default_progress_uses_charset_glyphs( progress=progress.append, ) - expected = f"Running SessionStart hook{get_glyphs().ellipsis}" - assert {update.message for update in progress} == {expected} - assert "…" not in expected - reset_glyphs_cache() - - -async def test_engine_progress_callback_raise_does_not_break_execution( - tmp_path: Path, -) -> None: - snapshot = HooksSnapshot.from_config( - _config( - { - "SessionStart": [ - { - "hooks": [ - { - "type": "command", - "command": "unused", - "argv": [sys.executable, "-c", "pass"], - "statusMessage": "Loading", - } - ] - } - ] - } - ) - ) - - def boom(_update: HookProgress) -> None: - msg = "progress sink failed" - raise RuntimeError(msg) - - decision = await HookEngine(snapshot).run( - _invocation( - tmp_path, - SessionStartEvent( - event=HookEvent.SESSION_START, - cause=SessionStartCause.STARTUP, - ), - ), - transcript_path=_transcript_path(tmp_path), - progress=boom, - ) - - assert isinstance(decision, SessionStartDecision) - assert decision.continue_processing is True - - -async def test_engine_clears_progress_on_cancellation(tmp_path: Path) -> None: - script = ( - "import sys,time; time.sleep(30); sys.stdout.write('{}'); sys.stdout.flush()" - ) - snapshot = HooksSnapshot.from_config( - _config( - { - "SessionStart": [ - { - "hooks": [ - { - "type": "command", - "command": "unused", - "argv": [sys.executable, "-c", script], - "statusMessage": "Slow hook", - "timeout": 60, - } - ] - } - ] - } - ) - ) - progress: list[HookProgress] = [] - task = asyncio.create_task( - HookEngine(snapshot).run( - _invocation( - tmp_path, - SessionStartEvent( - event=HookEvent.SESSION_START, - cause=SessionStartCause.STARTUP, - ), - ), - transcript_path=_transcript_path(tmp_path), - progress=progress.append, - ) - ) - for _ in range(50): - if progress and progress[-1].active: - break - await asyncio.sleep(0.01) - assert progress - assert progress[-1].active is True - task.cancel() - with pytest.raises(asyncio.CancelledError): - await task - assert progress[-1].active is False - assert progress[-1].message == "Slow hook" + assert [(update.active, update.message) for update in progress] == [ + (True, "Loading project context"), + (False, "Loading project context"), + ] async def test_engine_uses_captured_snapshot(tmp_path: Path) -> None: diff --git a/libs/code/tests/unit_tests/hooks/test_feedback.py b/libs/code/tests/unit_tests/hooks/test_feedback.py index 03248ded49a..e9704b53611 100644 --- a/libs/code/tests/unit_tests/hooks/test_feedback.py +++ b/libs/code/tests/unit_tests/hooks/test_feedback.py @@ -2,183 +2,30 @@ from __future__ import annotations -from io import StringIO -from typing import TYPE_CHECKING - from deepagents_code.hooks.feedback import HookFeedback, HookProgress -from deepagents_code.hooks.models.domain import ( - HookDiagnostic, - HookEvent, - PermissionEffect, - SessionEndDecision, -) - -if TYPE_CHECKING: - import pytest - - -def test_decision_feedback_scopes_diagnostics_per_invocation( - monkeypatch: pytest.MonkeyPatch, -) -> None: - notices: list[tuple[str, str]] = [] - output = StringIO() - monkeypatch.setattr("deepagents_code.hooks.feedback.sys.stdout", output) - feedback = HookFeedback( - notice=lambda message, severity: notices.append((message, severity)) - ) - diagnostic = HookDiagnostic( - code="invalid_output", - severity="warning", - message="Hook output failed validation", - handler_id="SessionEnd:0:0", - ) - decision = SessionEndDecision( - event=HookEvent.SESSION_END, - user_notices=["visible notice"], - terminal_sequences=["\x1b]9;done\x07"], - diagnostics=[diagnostic, diagnostic], - ) - - feedback.present_decision(decision) - feedback.present_decision( - SessionEndDecision( - event=HookEvent.SESSION_END, - diagnostics=[diagnostic], - ) - ) - - assert notices == [ - ("Hook warning: Hook output failed validation", "warning"), - ("visible notice", "information"), - ("Hook warning: Hook output failed validation", "warning"), - ] - assert output.getvalue() == "\x1b]9;done\x07" +from deepagents_code.hooks.models.domain import HookEvent -def test_failed_diagnostic_notice_remains_eligible_for_retry() -> None: - attempts = {"count": 0} - notices: list[str] = [] - - def flaky_notice(message: str, severity: str) -> None: - _ = severity - attempts["count"] += 1 - if attempts["count"] == 1: - msg = "sink unavailable" - raise RuntimeError(msg) - notices.append(message) - - feedback = HookFeedback(notice=flaky_notice) - diagnostic = HookDiagnostic( - code="invalid_output", - severity="warning", - message="Hook output failed validation", - ) - - feedback.present_diagnostics([diagnostic]) - feedback.present_diagnostics([diagnostic]) - - assert notices == ["Hook warning: Hook output failed validation"] - - -def test_progress_keeps_latest_concurrent_status_visible() -> None: - statuses: list[str] = [] - - def capture_status(message: str) -> None: - statuses.append(message) - - feedback = HookFeedback(status=capture_status) - first = HookProgress( - operation_id="first", - handler_id="Stop:0:0", +def _progress(operation_id: str, message: str, *, active: bool = True) -> HookProgress: + return HookProgress( + operation_id=operation_id, + handler_id=f"Stop:{operation_id}", event=HookEvent.STOP, - message="Checking output", - active=True, - ) - second = HookProgress( - operation_id="second", - handler_id="Stop:0:1", - event=HookEvent.STOP, - message="Running policy", - active=True, + message=message, + active=active, ) - feedback.update_progress(first) - feedback.update_progress(second) - feedback.update_progress( - HookProgress( - operation_id=first.operation_id, - handler_id=first.handler_id, - event=first.event, - message=first.message, - active=False, - ) - ) - feedback.update_progress( - HookProgress( - operation_id=second.operation_id, - handler_id=second.handler_id, - event=second.event, - message=second.message, - active=False, - ) - ) - - assert statuses == [ - "Checking output", - "Running policy", - "Running policy", - "", - ] - -def test_progress_callback_raise_does_not_break_updates() -> None: +def test_progress_keeps_latest_concurrent_status_visible() -> None: statuses: list[str] = [] + feedback = HookFeedback(status=lambda message: statuses.append(message)) - def flaky_status(message: str) -> None: - if message == "boom": - msg = "status sink failed" - raise RuntimeError(msg) - statuses.append(message) - - feedback = HookFeedback(status=flaky_status) - feedback.update_progress( - HookProgress( - operation_id="one", - handler_id="Stop:0:0", - event=HookEvent.STOP, - message="boom", - active=True, - ) - ) - feedback.update_progress( - HookProgress( - operation_id="one", - handler_id="Stop:0:0", - event=HookEvent.STOP, - message="recovered", - active=True, - ) - ) - - assert statuses == ["recovered"] - - -def test_permission_feedback_attributes_hook_decisions() -> None: - notices: list[tuple[str, str]] = [] - feedback = HookFeedback( - notice=lambda message, severity: notices.append((message, severity)) - ) - - feedback.present_permission("read_file", PermissionEffect(behavior="allow")) - feedback.present_permission( - "execute", - PermissionEffect(behavior="deny", reason="command blocked"), - ) + for update in ( + _progress("first", "Checking output"), + _progress("second", "Running policy"), + _progress("first", "Checking output", active=False), + _progress("second", "Running policy", active=False), + ): + feedback.update_progress(update) - assert notices == [ - ("PermissionRequest hook allowed read_file.", "information"), - ( - "PermissionRequest hook denied execute: command blocked", - "warning", - ), - ] + assert statuses == ["Checking output", "Running policy", "Running policy", ""] diff --git a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py index 48a7b81e188..199600c1d34 100644 --- a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py @@ -6,7 +6,6 @@ import json import sys from datetime import UTC, datetime, timedelta -from io import StringIO from pathlib import Path from typing import TYPE_CHECKING, Any from unittest.mock import MagicMock @@ -830,35 +829,11 @@ async def handler(_request: object) -> ToolMessage: async def test_fulfill_hook_invocation_runs_engine(tmp_path: Path) -> None: config_dir = tmp_path / "config" config_dir.mkdir() - command = "import json; print(json.dumps({'systemMessage': 'visible notice'}))" - (config_dir / "hooks.json").write_text( - json.dumps( - { - "hooks": { - "PreToolUse": [ - { - "hooks": [ - { - "type": "command", - "command": "unused", - "argv": [sys.executable, "-c", command], - } - ] - } - ] - } - } - ), - encoding="utf-8", - ) - notices: list[tuple[str, str]] = [] + (config_dir / "hooks.json").write_text('{"hooks":{}}', encoding="utf-8") runtime = HooksRuntime.create( cwd=tmp_path, config_dir=config_dir, transcript_root=tmp_path / "transcripts", - feedback=HookFeedback( - notice=lambda message, severity: notices.append((message, severity)) - ), ) request = _request() request = request.model_copy(update={"snapshot_id": runtime.snapshot_id}) @@ -871,12 +846,10 @@ async def test_fulfill_hook_invocation_runs_engine(tmp_path: Path) -> None: ) assert isinstance(response.decision, PreToolUseDecision) assert response.decision.permission.behavior in {"allow", "none"} - assert notices == [("visible notice", "information")] async def test_fulfillment_is_idempotent_in_flight_and_after_completion( tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, ) -> None: config_dir = tmp_path / "config" config_dir.mkdir() @@ -885,14 +858,7 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( "import json,pathlib,time; " f"pathlib.Path({str(marker)!r}).write_text('x'); " "time.sleep(0.05); " - "print(json.dumps({" - "'systemMessage':'once'," - "'terminalSequence':'\\u0007'," - "'hookSpecificOutput':{" - "'hookEventName':'PreToolUse'," - "'permissionDecision':'allow'" - "}" - "}))" + "print(json.dumps({'systemMessage':'once'}))" ) (config_dir / "hooks.json").write_text( json.dumps( @@ -916,10 +882,6 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( encoding="utf-8", ) notices: list[tuple[str, str]] = [] - output = StringIO() - monkeypatch.setattr("deepagents_code.hooks.feedback.sys.stdout", output) - # Force a terminal sequence through a decision that includes one by patching - # after invoke would be heavy; instead assert notice exactly-once via sink. runtime = HooksRuntime.create( cwd=tmp_path, config_dir=config_dir, @@ -937,8 +899,7 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( assert first == second == third assert marker.read_text() == "x" - assert notices.count(("once", "information")) == 1 - assert output.getvalue() == "\a" + assert notices == [("once", "information")] def test_snapshot_configured_server_events() -> None: diff --git a/libs/code/tests/unit_tests/tui/widgets/test_status.py b/libs/code/tests/unit_tests/tui/widgets/test_status.py index 7c817eb9163..975bf818b8b 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_status.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_status.py @@ -530,30 +530,15 @@ async def test_hook_and_agent_status_do_not_clobber(self) -> None: msg = pilot.app.query_one("#status-message", Static) bar.set_status_message("Loading thread", source="agent") - await pilot.pause() - assert str(msg.render()) == "Loading thread" - - bar.set_status_message("Running [lint] checks", source="hooks") - await pilot.pause() - assert str(msg.render()) == "Running [lint] checks" - + bar.set_status_message("Running [bold]hook[/bold]", source="hooks") bar.set_status_message("Still loading", source="agent") await pilot.pause() - assert str(msg.render()) == "Running [lint] checks" + assert str(msg.render()) == "Running [bold]hook[/bold]" bar.set_status_message("", source="hooks") await pilot.pause() assert str(msg.render()) == "Still loading" - async def test_status_message_renders_markup_literally(self) -> None: - """Configured status text must not be interpreted as Rich markup.""" - async with StatusBarApp().run_test() as pilot: - bar = pilot.app.query_one("#status-bar", StatusBar) - msg = pilot.app.query_one("#status-message", Static) - bar.set_status_message("Running [bold]hook[/bold]", source="hooks") - await pilot.pause() - assert str(msg.render()) == "Running [bold]hook[/bold]" - async def test_busy_shows_slot_and_clearing_hides(self) -> None: """A busy indicator reveals the slot; clearing busy (no message) hides it.""" async with StatusBarApp().run_test() as pilot: From ed8793a39260282634fe200412f63681d64e734e Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 29 Jul 2026 11:39:22 -0700 Subject: [PATCH 4/6] cr --- libs/code/deepagents_code/app.py | 21 ++-- .../deepagents_code/client/non_interactive.py | 46 ++++++-- libs/code/deepagents_code/hooks/client.py | 2 +- .../deepagents_code/hooks/client_lifecycle.py | 41 ++----- libs/code/deepagents_code/hooks/engine.py | 36 +++--- libs/code/deepagents_code/hooks/manager.py | 106 +++++++++--------- .../hooks/{feedback.py => presenter.py} | 55 +++++++-- libs/code/deepagents_code/hooks/runtime.py | 13 ++- .../unit_tests/hooks/test_client_lifecycle.py | 12 +- .../tests/unit_tests/hooks/test_engine.py | 4 +- .../tests/unit_tests/hooks/test_feedback.py | 31 ----- .../tests/unit_tests/hooks/test_manager.py | 94 ++++++++++++++++ .../tests/unit_tests/hooks/test_presenter.py | 80 +++++++++++++ .../unit_tests/hooks/test_server_lifecycle.py | 4 +- .../code/tests/unit_tests/hooks/test_trust.py | 1 - libs/code/tests/unit_tests/test_app.py | 1 - .../tests/unit_tests/test_non_interactive.py | 2 - libs/code/tests/unit_tests/test_offload.py | 1 - 18 files changed, 366 insertions(+), 184 deletions(-) rename libs/code/deepagents_code/hooks/{feedback.py => presenter.py} (74%) delete mode 100644 libs/code/tests/unit_tests/hooks/test_feedback.py create mode 100644 libs/code/tests/unit_tests/hooks/test_manager.py create mode 100644 libs/code/tests/unit_tests/hooks/test_presenter.py diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index ccf6831bdd7..3af137cb700 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -601,11 +601,11 @@ class _ConfigWriteResult: from deepagents_code.config_manifest import CursorStyle from deepagents_code.event_bus import EventSource, ExternalEvent from deepagents_code.goal_rubric import GoalCreateRequest, GoalCriteriaRequest - from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager from deepagents_code.hooks.models.domain import ( SessionStartCause, ) + from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter from deepagents_code.hooks.trust import WorkspaceTrust from deepagents_code.mcp_tools import MCPServerInfo from deepagents_code.model_config import MissingProviderPackageError @@ -2283,7 +2283,6 @@ def __init__( self.hooks: HooksManager = HooksManager.adopting( None, identity=self.hook_identity, - notice=lambda _message: None, ) """Client-side Hooks v2 coordinator. @@ -4368,10 +4367,10 @@ async def _post_paint_init(self) -> None: lambda: asyncio.create_task(self._run_session_start_sequence()), ) - def _create_hook_feedback(self) -> HookFeedback: - from deepagents_code.hooks.feedback import HookFeedback + def _create_hook_presenter(self) -> HookPresenter: + from deepagents_code.hooks.presenter import HookPresenter - return HookFeedback( + return HookPresenter( notice=self._notify_hook_feedback, status=self._update_hook_status, ) @@ -4379,7 +4378,7 @@ def _create_hook_feedback(self) -> HookFeedback: def _notify_hook_feedback( self, message: str, - severity: HookFeedbackSeverity, + severity: HookNoticeSeverity, ) -> None: self.notify(message, severity=severity, markup=False) @@ -4423,9 +4422,8 @@ async def _init_session_state(self) -> None: session_state.hooks = HooksManager.create( cwd=Path(self._cwd), identity=session_state.hook_identity, - notice=lambda message: self.notify(message, markup=False), + presenter=self._create_hook_presenter(), trust=self._hook_trust, - feedback=self._create_hook_feedback(), ) # Re-read the app-owned selection last so a mode change during # construction cannot be overwritten by the freshly built state. @@ -4445,7 +4443,7 @@ def _hooks(self) -> HooksManager: self._detached_hooks = HooksManager.adopting( None, identity=self._hook_identity, - notice=lambda message: self.notify(message, markup=False), + presenter=self._create_hook_presenter(), ) return self._detached_hooks @@ -4465,10 +4463,7 @@ async def _reload_hooks(self) -> None: """ from pathlib import Path - await self._hooks.reload( - cwd=Path(self._cwd), - feedback=self._create_hook_feedback(), - ) + await self._hooks.reload(cwd=Path(self._cwd)) async def _run_session_start_hook(self, cause: SessionStartCause) -> bool: """Run `SessionStart`, surfacing a stop as a chat message. diff --git a/libs/code/deepagents_code/client/non_interactive.py b/libs/code/deepagents_code/client/non_interactive.py index 3d62c767aa3..06c2eb42b3e 100644 --- a/libs/code/deepagents_code/client/non_interactive.py +++ b/libs/code/deepagents_code/client/non_interactive.py @@ -91,6 +91,10 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.manager import HooksManager from deepagents_code.hooks.models.domain import SessionEndCause + from deepagents_code.hooks.presenter import ( + HookNoticeSeverity, + HookPresenter, + ) from deepagents_code.hooks.transcript import TranscriptRecorder logger = logging.getLogger(__name__) @@ -351,6 +355,27 @@ def _inert_hooks() -> HooksManager: return HooksManager.inert() +def _plain_hook_presenter(console: Console) -> HookPresenter: + """Build a presenter that prints notices plainly and shows no status. + + Used for hooks loaded before the run owns a spinner; `attach_output` later + rebinds this same presenter to the styled, spinner-aware sinks. + + Args: + console: Destination for notice text. + + Returns: + A presenter with an unstyled notice sink. + """ + from deepagents_code.hooks.presenter import HookPresenter + + def notice(message: str, severity: HookNoticeSeverity) -> None: + del severity + console.print(Text(message), highlight=False) + + return HookPresenter(notice=notice) + + @dataclass class StreamState: """Mutable state accumulated while iterating over the agent stream.""" @@ -1357,7 +1382,6 @@ async def _run_agent_loop( from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.client_lifecycle import ClientHookStopError - from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager from deepagents_code.hooks.models.domain import ( DcodeNotificationKind, @@ -1365,13 +1389,14 @@ async def _run_agent_loop( SessionEndCause, SessionStartCause, ) + from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.trust import WorkspaceTrust hook_owned_spinner = False def present_hook_notice( message: str, - severity: HookFeedbackSeverity, + severity: HookNoticeSeverity, ) -> None: style = ( "bold red" @@ -1398,10 +1423,7 @@ def update_hook_status(message: str) -> None: elif spinner.is_running: spinner.update("Working...") - feedback = HookFeedback( - notice=present_hook_notice, - status=update_hook_status if spinner is not None else None, - ) + hook_status = update_hook_status if spinner is not None else None resolved_approval_mode = approval_mode or ApprovalMode.MANUAL # One headless turn per process, so identity is fixed for the whole run. identity = HookSessionIdentity( @@ -1411,12 +1433,15 @@ def update_hook_status(message: str) -> None: ) if hooks is not None: state.hooks = hooks - state.hooks.attach_feedback(feedback) + state.hooks.attach_output(notice=present_hook_notice, status=hook_status) else: state.hooks = HooksManager.create( cwd=Path.cwd(), identity=lambda: identity, - notice=lambda notice: console.print(Text(notice), highlight=False), + presenter=HookPresenter( + notice=present_hook_notice, + status=hook_status, + ), # Project hooks require an explicit opt-in, matching `--trust-project-mcp`. # Persisted interactive trust deliberately does not carry into headless # runs, so CI never inherits a grant made at someone's terminal. @@ -1424,7 +1449,6 @@ def update_hook_status(message: str) -> None: Path.cwd(), granted=trust_project_hooks, ), - feedback=feedback, ) state.hooks.apply_graph_context(context) context["approval_mode"] = resolved_approval_mode.value @@ -2029,7 +2053,9 @@ def discover_all_skills() -> tuple[list[ExtendedSkillMetadata], list[Path]]: hooks = HooksManager.create( cwd=Path.cwd(), identity=lambda: identity, - notice=lambda notice: console.print(Text(notice), highlight=False), + # Plain output until `_run_agent_loop` attaches the styled, + # spinner-aware sinks to this same presenter. + presenter=_plain_hook_presenter(console), # Explicit opt-in only; see `_run_agent_loop` for the rationale. trust=WorkspaceTrust.explicit_only(Path.cwd(), granted=trust_project_hooks), ) diff --git a/libs/code/deepagents_code/hooks/client.py b/libs/code/deepagents_code/hooks/client.py index 72d855e26e9..eb775d3a586 100644 --- a/libs/code/deepagents_code/hooks/client.py +++ b/libs/code/deepagents_code/hooks/client.py @@ -92,7 +92,7 @@ async def fulfill_hook_invocation( async def execute() -> HookInvocationResponse: decision = await runtime.invoke(request.invocation) - runtime.feedback.present_decision(decision) + runtime.presenter.present_decision(decision) return HookInvocationResponse( protocol_version=1, invocation_id=request.invocation_id, diff --git a/libs/code/deepagents_code/hooks/client_lifecycle.py b/libs/code/deepagents_code/hooks/client_lifecycle.py index 20871d414dc..4f6fe39fa65 100644 --- a/libs/code/deepagents_code/hooks/client_lifecycle.py +++ b/libs/code/deepagents_code/hooks/client_lifecycle.py @@ -7,7 +7,6 @@ from uuid import UUID from deepagents_code.approval_mode import ApprovalMode -from deepagents_code.hooks.feedback import HookFeedback, HookFeedbackSeverity from deepagents_code.hooks.models.domain import ( CompactTrigger, DcodeNotification, @@ -40,16 +39,17 @@ ) if TYPE_CHECKING: - from collections.abc import Callable from pathlib import Path from typing import Protocol + from deepagents_code.hooks.presenter import HookPresenter + class _ClientHooksRuntime(Protocol): @property def cwd(self) -> Path: ... @property - def feedback(self) -> HookFeedback: ... + def presenter(self) -> HookPresenter: ... def configured_events(self) -> frozenset[HookEvent]: ... @@ -107,34 +107,17 @@ def create( @dataclass(slots=True) class ClientHookService: - """Execute client-owned events and apply their common side effects.""" + """Execute client-owned events and apply their common side effects. + + User-facing output goes through the runtime's presenter, which the owning + `HooksManager` also holds. The service never wraps or replaces it, so there + is exactly one presenter per session. + """ runtime: _ClientHooksRuntime - notice: Callable[[str], None] | None = None - feedback: HookFeedback | None = None # SessionStart context accumulated per thread, consumed by # `take_session_context` for injection into the next model turn. _pending_context: dict[str, list[str]] = field(default_factory=dict) - _feedback: HookFeedback = field(init=False) - - def __post_init__(self) -> None: - """Resolve one presenter shared with the session runtime when available.""" - if self.feedback is not None: - self._feedback = self.feedback - elif self.notice is not None: - notice = self.notice - - def sink(message: str, severity: HookFeedbackSeverity) -> None: - del severity - notice(message) - - base = self.runtime.feedback - self._feedback = HookFeedback( - notice=sink, - status=base.status, - ) - else: - self._feedback = self.runtime.feedback async def session_start( self, @@ -311,7 +294,7 @@ async def resolve_permission( The returned HITL decision carries the raw hook reason (or stop reason) for model-visible resume payloads. Attribution text is emitted only - through the shared feedback presenter. + through the shared presenter. Args: context: Current client session context. @@ -413,7 +396,7 @@ def present_permission( tool_name: Display name of the affected tool. permission: Normalized permission effect. """ - self._feedback.present_permission(tool_name, permission) + self.runtime.presenter.present_permission(tool_name, permission) async def _invoke( self, @@ -430,5 +413,5 @@ async def _invoke( event=event, ) decision = await self.runtime.invoke(invocation) - self._feedback.present_decision(decision) + self.runtime.presenter.present_decision(decision) return decision diff --git a/libs/code/deepagents_code/hooks/engine.py b/libs/code/deepagents_code/hooks/engine.py index abbc8572208..14f013d482a 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -9,8 +9,8 @@ from deepagents_code.hooks.capabilities import get_event_spec from deepagents_code.hooks.envelope import HookEnvelopeAdapter -from deepagents_code.hooks.feedback import HookProgress from deepagents_code.hooks.models.domain import HookDiagnostic +from deepagents_code.hooks.presenter import HookProgress from deepagents_code.hooks.runner import ( MAX_HOOK_OUTPUT_BYTES, HandlerResult, @@ -27,12 +27,6 @@ logger = logging.getLogger(__name__) -def _default_progress_message(handler: HookHandler) -> str: - from deepagents_code.config import get_glyphs - - return f"Running {handler.event.value} hook{get_glyphs().ellipsis}" - - @dataclass(frozen=True, slots=True) class HookEngine: """Execute Hooks v2 invocations against one immutable snapshot.""" @@ -48,7 +42,7 @@ async def run( *, transcript_path: Path, agent_transcript_path: Path | None = None, - progress: Callable[[HookProgress], None] | None = None, + on_progress: Callable[[HookProgress], None] | None = None, ) -> HookDecision: """Execute matching handlers and return a normalized decision. @@ -60,7 +54,7 @@ async def run( invocation: Native lifecycle invocation. transcript_path: Materialized client transcript path. agent_transcript_path: Materialized subagent transcript path. - progress: Optional handler lifecycle callback. + on_progress: Optional handler lifecycle callback. Returns: The event-specific decision produced by ordered hook reduction. @@ -103,7 +97,7 @@ async def run( default_timeout=event_default, max_output_bytes=self.max_output_bytes, operation_id=f"{id(invocation):x}:{handler.id}", - progress=progress, + on_progress=on_progress, ) for handler in match.handlers ) @@ -126,19 +120,19 @@ async def _run_handler( default_timeout: float, max_output_bytes: int, operation_id: str, - progress: Callable[[HookProgress], None] | None, + on_progress: Callable[[HookProgress], None] | None, ) -> HandlerResult: message = (handler.status_message or "").strip() - if not message: - message = _default_progress_message(handler) - update = HookProgress( - operation_id=operation_id, - handler_id=handler.id, - event=handler.event, - message=message, - active=True, + _report_progress( + on_progress, + HookProgress( + operation_id=operation_id, + handler_id=handler.id, + event=handler.event, + message=message, + active=True, + ), ) - _report_progress(progress, update) try: return await run_command_handler( handler, @@ -149,7 +143,7 @@ async def _run_handler( ) finally: _report_progress( - progress, + on_progress, HookProgress( operation_id=operation_id, handler_id=handler.id, diff --git a/libs/code/deepagents_code/hooks/manager.py b/libs/code/deepagents_code/hooks/manager.py index f190bb64774..2d96d3f1e36 100644 --- a/libs/code/deepagents_code/hooks/manager.py +++ b/libs/code/deepagents_code/hooks/manager.py @@ -26,6 +26,7 @@ PermissionHookOutcome, PermissionPlan, ) +from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.trust import WorkspaceTrust if TYPE_CHECKING: @@ -37,7 +38,6 @@ from deepagents_code._cli_context import CLIContext from deepagents_code.approval_mode import ApprovalMode - from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.models.domain import ( CompactTrigger, DcodeNotificationKind, @@ -45,6 +45,10 @@ SessionStartCause, ToolCallData, ) + from deepagents_code.hooks.presenter import ( + HookNoticeCallback, + HookStatusCallback, + ) from deepagents_code.hooks.runtime import HooksRuntime from deepagents_code.hooks.transcript import TranscriptRecorder @@ -99,10 +103,15 @@ def append_messages( @dataclass(slots=True) class HooksManager: - """Owns the Hooks v2 runtime, hook service, and transcript projection.""" + """Owns the Hooks v2 runtime, presenter, hook service, and transcripts. + + The presenter is the manager's, not the runtime's: one instance is created + once and handed to every runtime the manager loads, so a reload or a late + UI attachment never leaves two presenters competing for the same output. + """ identity: SessionIdentityProvider - notice: Callable[[str], None] + presenter: HookPresenter = field(default_factory=HookPresenter) _runtime: HooksRuntime | None = None trust: WorkspaceTrust = field(default_factory=WorkspaceTrust) """Policy re-resolved on every reload; the manager is its only interpreter.""" @@ -119,9 +128,8 @@ def create( *, cwd: Path, identity: SessionIdentityProvider, - notice: Callable[[str], None], + presenter: HookPresenter | None = None, trust: WorkspaceTrust | None = None, - feedback: HookFeedback | None = None, ) -> HooksManager: """Load hook configuration and return a ready manager. @@ -131,19 +139,20 @@ def create( Args: cwd: Session working directory used to resolve hook configuration. identity: Reads current thread, approval mode, and prompt id. - notice: Surfaces hook `systemMessage` notices to the user. + presenter: Presents notices, diagnostics, and progress. When + omitted, a sinkless one is created and output is only logged + until `attach_output` binds it to a UI. trust: Project-hook trust policy. Defaults to trusting nothing beyond what the persisted trust store already records. - feedback: Shared presenter for notices, diagnostics, and progress. - When omitted, feedback is logged rather than surfaced. Returns: A manager owning the loaded runtime, or an inert one on failure. """ policy = trust if trust is not None else WorkspaceTrust.none() - runtime = _load_runtime(cwd, trust=policy, feedback=feedback) + resolved = presenter if presenter is not None else HookPresenter() + runtime = _load_runtime(cwd, trust=policy, presenter=resolved) _present_load_diagnostics(runtime) - return cls(identity, notice, runtime, policy) + return cls(identity, resolved, runtime, policy) @classmethod def adopting( @@ -151,19 +160,32 @@ def adopting( runtime: HooksRuntime | None, *, identity: SessionIdentityProvider, - notice: Callable[[str], None], + presenter: HookPresenter | None = None, ) -> HooksManager: """Wrap an already-loaded runtime. + A supplied runtime brought its own presenter, so that one is adopted + rather than displaced; `presenter`'s sinks are copied onto it to keep a + single instance. + Args: runtime: Preloaded runtime, or `None` when loading failed. identity: Reads current thread, approval mode, and prompt id. - notice: Surfaces hook `systemMessage` notices to the user. + presenter: Presents notices, diagnostics, and progress. Returns: A manager owning `runtime`. """ - return cls(identity, notice, runtime) + if runtime is None: + return cls( + identity, presenter if presenter is not None else HookPresenter() + ) + if presenter is not None: + runtime.presenter.attach( + notice=presenter.notice, + status=presenter.status, + ) + return cls(identity, runtime.presenter, runtime) @classmethod def inert(cls) -> HooksManager: @@ -174,29 +196,26 @@ def inert(cls) -> HooksManager: """ from deepagents_code.approval_mode import ApprovalMode - return cls( - lambda: HookSessionIdentity("", ApprovalMode.MANUAL), - lambda _message: None, - None, - ) + return cls(lambda: HookSessionIdentity("", ApprovalMode.MANUAL)) - def attach_feedback(self, feedback: HookFeedback) -> None: - """Route hook notices, diagnostics, and progress through `feedback`. + def attach_output( + self, + *, + notice: HookNoticeCallback | None, + status: HookStatusCallback | None = None, + ) -> None: + """Route hook notices, diagnostics, and progress to a client's UI. For callers handed a manager that was loaded before their UI existed. Load diagnostics are re-presented so anything the earlier load could only log now reaches the user. Args: - feedback: Presenter to adopt. + notice: Sink for user-visible notices. + status: Sink for transient hook-owned status text. """ - runtime = self._runtime - if runtime is None: - return - runtime.feedback.notice = feedback.notice - runtime.feedback.status = feedback.status - self._service = self._build_service() - _present_load_diagnostics(runtime) + self.presenter.attach(notice=notice, status=status) + _present_load_diagnostics(self._runtime) @property def enabled(self) -> bool: @@ -215,36 +234,27 @@ def has_handlers(self, event: HookEvent) -> bool: service = self._service return service is not None and service.has_handlers(event) - async def reload( - self, - *, - cwd: Path, - feedback: HookFeedback | None = None, - ) -> None: + async def reload(self, *, cwd: Path) -> None: """Rebuild the runtime after the session working directory changes. Workspace trust is re-resolved for `cwd`, so moving from a trusted project into an untrusted one drops project hooks instead of carrying - the previous grant forward. + the previous grant forward. The presenter survives the reload, so the + client's output sinks stay bound. Pending `SessionStart` context is dropped with the old runtime, matching the lifecycle boundary that triggers a reload. Args: cwd: New session working directory. - feedback: Shared presenter for notices, diagnostics, and progress. - When omitted, the previous runtime's presenter is preserved. """ import asyncio - existing_feedback = ( - self._runtime.feedback if self._runtime is not None else None - ) self._runtime = await asyncio.to_thread( _load_runtime, cwd, trust=self.trust, - feedback=feedback if feedback is not None else existing_feedback, + presenter=self.presenter, ) self._service = self._build_service() _present_load_diagnostics(self._runtime) @@ -556,11 +566,7 @@ def _build_service(self) -> ClientHookService | None: runtime = self._runtime if runtime is None: return None - # Prefer the shared presenter so notices keep their severity and hook - # progress reaches the status bar. `notice` remains the fallback for - # callers that never supplied one. - presenter = runtime.feedback if runtime.feedback.notice is not None else None - return ClientHookService(runtime, notice=self.notice, feedback=presenter) + return ClientHookService(runtime) def _context(self, *, thread_id: str | None = None) -> ClientHookContext: identity = self.identity() @@ -579,14 +585,14 @@ def _present_load_diagnostics(runtime: HooksRuntime | None) -> None: """ if runtime is None: return - runtime.feedback.present_diagnostics(runtime.snapshot.diagnostics) + runtime.presenter.present_diagnostics(runtime.snapshot.diagnostics) def _load_runtime( cwd: Path, *, trust: WorkspaceTrust, - feedback: HookFeedback | None = None, + presenter: HookPresenter, ) -> HooksRuntime | None: """Resolve workspace trust for `cwd` and load a runtime under it. @@ -596,7 +602,7 @@ def _load_runtime( Args: cwd: Session working directory. trust: Policy deciding whether project hooks may load. - feedback: Shared presenter for notices, diagnostics, and progress. + presenter: The manager's presenter, shared with the new runtime. Returns: The loaded runtime, or `None` when configuration could not be loaded. @@ -607,7 +613,7 @@ def _load_runtime( return HooksRuntime.create( cwd=cwd, workspace_trusted=trust.allows(cwd), - feedback=feedback, + presenter=presenter, ) except Exception: logger.exception("Failed to load hook configuration; hooks disabled") diff --git a/libs/code/deepagents_code/hooks/feedback.py b/libs/code/deepagents_code/hooks/presenter.py similarity index 74% rename from libs/code/deepagents_code/hooks/feedback.py rename to libs/code/deepagents_code/hooks/presenter.py index 35fb1172927..8c78ca29ca4 100644 --- a/libs/code/deepagents_code/hooks/feedback.py +++ b/libs/code/deepagents_code/hooks/presenter.py @@ -1,4 +1,10 @@ -"""Shared user-facing feedback for Hooks v2 execution.""" +"""User-facing presentation for Hooks v2 execution. + +`HookPresenter` is the single place that turns hook results into something a +person sees. It is owned by `HooksManager`, handed to every runtime that +manager loads, and kept alive across reloads so its output sinks can be +rebound once a UI exists without any other object holding its own copy. +""" from __future__ import annotations @@ -19,14 +25,14 @@ logger = logging.getLogger(__name__) -HookFeedbackSeverity: TypeAlias = Literal["information", "warning", "error"] +HookNoticeSeverity: TypeAlias = Literal["information", "warning", "error"] DiagnosticKey: TypeAlias = tuple[str, str, str, str | None, str | None] class HookNoticeCallback(Protocol): """Callable that surfaces a user-visible hook notice.""" - def __call__(self, message: str, severity: HookFeedbackSeverity) -> None: + def __call__(self, message: str, severity: HookNoticeSeverity) -> None: """Present one notice to the user. Args: @@ -53,18 +59,37 @@ class HookProgress: operation_id: str handler_id: str event: HookEvent - message: str active: bool + message: str = "" + """Handler-authored status text. Empty when the handler supplied none.""" @dataclass(slots=True) -class HookFeedback: - """Present hook feedback consistently across interactive and headless clients.""" +class HookPresenter: + """Present hook output consistently across interactive and headless clients.""" notice: HookNoticeCallback | None = None status: HookStatusCallback | None = None _active_statuses: dict[str, str] = field(default_factory=dict) + def attach( + self, + *, + notice: HookNoticeCallback | None, + status: HookStatusCallback | None = None, + ) -> None: + """Rebind the output sinks without replacing the presenter. + + Lets a client that loaded hooks before its UI existed start surfacing + output, while every runtime and service keeps the same instance. + + Args: + notice: Sink for user-visible notices. + status: Sink for transient hook-owned status text. + """ + self.notice = notice + self.status = status + def present_decision(self, decision: HookDecision) -> None: """Present common side effects from a reduced hook decision. @@ -104,7 +129,7 @@ def present_diagnostics(self, diagnostics: Iterable[HookDiagnostic]) -> None: ) if key in delivered: continue - severity: HookFeedbackSeverity = ( + severity: HookNoticeSeverity = ( "error" if diagnostic.severity == "error" else "warning" ) if self._notify(f"Hook {severity}: {diagnostic.message}", severity): @@ -121,7 +146,7 @@ def update_progress(self, progress: HookProgress) -> None: progress: Handler lifecycle update. """ if progress.active: - self._active_statuses[progress.operation_id] = progress.message + self._active_statuses[progress.operation_id] = _status_text(progress) else: self._active_statuses.pop(progress.operation_id, None) message = next(reversed(self._active_statuses.values()), "") @@ -154,14 +179,14 @@ def present_permission( "warning", ) - def _notify(self, message: str, severity: HookFeedbackSeverity) -> bool: + def _notify(self, message: str, severity: HookNoticeSeverity) -> bool: if self.notice is None: - logger.warning("Hook user feedback: %s", message) + logger.warning("Hook notice (no sink attached): %s", message) return True try: self.notice(message, severity) except Exception: - logger.warning("Failed to surface hook feedback", exc_info=True) + logger.warning("Failed to surface hook notice", exc_info=True) return False return True @@ -174,6 +199,14 @@ def _set_status(self, message: str) -> None: logger.warning("Failed to update hook status", exc_info=True) +def _status_text(progress: HookProgress) -> str: + if progress.message: + return progress.message + from deepagents_code.config import get_glyphs + + return f"Running {progress.event.value} hook{get_glyphs().ellipsis}" + + def _log_diagnostic(diagnostic: HookDiagnostic) -> None: message = "Hook diagnostic %s: %s" if diagnostic.severity == "error": diff --git a/libs/code/deepagents_code/hooks/runtime.py b/libs/code/deepagents_code/hooks/runtime.py index a829b0a27e6..6d76a105a70 100644 --- a/libs/code/deepagents_code/hooks/runtime.py +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -10,7 +10,6 @@ from deepagents_code.hooks.client import HookFulfillmentLedger from deepagents_code.hooks.engine import HookEngine -from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.loading import load_hooks_config from deepagents_code.hooks.models.domain import ( HookDecision, @@ -19,6 +18,7 @@ SubagentStartEvent, SubagentStopEvent, ) +from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.snapshot import HooksSnapshot from deepagents_code.hooks.transcript import TranscriptStore from deepagents_code.model_config import DEFAULT_CONFIG_DIR @@ -63,7 +63,7 @@ class HooksRuntime: """ project_hooks_loaded: bool - feedback: HookFeedback + presenter: HookPresenter fulfillments: HookFulfillmentLedger @classmethod @@ -74,7 +74,7 @@ def create( workspace_trusted: bool = False, config_dir: Path | None = None, transcript_root: Path | None = None, - feedback: HookFeedback | None = None, + presenter: HookPresenter | None = None, ) -> HooksRuntime: """Load configuration once and freeze a session runtime. @@ -88,7 +88,8 @@ def create( Defaults to `~/.deepagents/transcripts` regardless of `config_dir` (project and test hook configs must not relocate the global transcript store). - feedback: Shared user-facing feedback presenter. + presenter: Shared user-facing presenter. A private one is created + when omitted, so output is logged rather than surfaced. Returns: A runtime ready to execute invocations for this session. @@ -118,7 +119,7 @@ def create( cwd=project_context.user_cwd, workspace_trusted=workspace_trusted, project_hooks_loaded=loaded.project_source_loaded, - feedback=feedback or HookFeedback(), + presenter=presenter if presenter is not None else HookPresenter(), fulfillments=HookFulfillmentLedger(), ) @@ -181,7 +182,7 @@ async def invoke(self, invocation: HookInvocation) -> HookDecision: prepared.invocation, transcript_path=prepared.transcript_path, agent_transcript_path=prepared.agent_transcript_path, - progress=self.feedback.update_progress, + on_progress=self.presenter.update_progress, ) def prepare_invocation( diff --git a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py index 0a23d41e771..44b64b2ee75 100644 --- a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py @@ -15,7 +15,6 @@ ClientHookService, ClientHookStopError, ) -from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.models.domain import ( DcodeNotificationKind, HookDecision, @@ -31,6 +30,7 @@ SessionStartDecision, ) from deepagents_code.hooks.permissions import permission_hook_outcome +from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter if TYPE_CHECKING: from pathlib import Path @@ -41,7 +41,7 @@ class _Runtime: cwd: Path decisions: deque[HookDecision] invocations: list[HookInvocation] = field(default_factory=list) - feedback: HookFeedback = field(default_factory=HookFeedback) + presenter: HookPresenter = field(default_factory=HookPresenter) def configured_events(self) -> frozenset[HookEvent]: return frozenset(decision.event for decision in self.decisions) @@ -85,7 +85,13 @@ async def test_common_effects_context_and_live_hook_fields( ), ) notices: list[str] = [] - service = ClientHookService(runtime, notice=notices.append) + + def record(message: str, severity: HookNoticeSeverity) -> None: + del severity + notices.append(message) + + runtime.presenter.attach(notice=record) + service = ClientHookService(runtime) decision = await service.session_start( _context(prompt_id=prompt_id), SessionStartCause.STARTUP diff --git a/libs/code/tests/unit_tests/hooks/test_engine.py b/libs/code/tests/unit_tests/hooks/test_engine.py index 2f282d771c3..c7cf764f72f 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -62,8 +62,8 @@ if TYPE_CHECKING: from pathlib import Path - from deepagents_code.hooks.feedback import HookProgress from deepagents_code.hooks.models.domain import HookDomainEvent + from deepagents_code.hooks.presenter import HookProgress from deepagents_code.json_types import JsonObject @@ -1655,7 +1655,7 @@ async def test_engine_reports_configured_handler_status(tmp_path: Path) -> None: ), ), transcript_path=_transcript_path(tmp_path), - progress=progress.append, + on_progress=progress.append, ) assert [(update.active, update.message) for update in progress] == [ diff --git a/libs/code/tests/unit_tests/hooks/test_feedback.py b/libs/code/tests/unit_tests/hooks/test_feedback.py deleted file mode 100644 index e9704b53611..00000000000 --- a/libs/code/tests/unit_tests/hooks/test_feedback.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Tests for shared Hooks v2 user feedback.""" - -from __future__ import annotations - -from deepagents_code.hooks.feedback import HookFeedback, HookProgress -from deepagents_code.hooks.models.domain import HookEvent - - -def _progress(operation_id: str, message: str, *, active: bool = True) -> HookProgress: - return HookProgress( - operation_id=operation_id, - handler_id=f"Stop:{operation_id}", - event=HookEvent.STOP, - message=message, - active=active, - ) - - -def test_progress_keeps_latest_concurrent_status_visible() -> None: - statuses: list[str] = [] - feedback = HookFeedback(status=lambda message: statuses.append(message)) - - for update in ( - _progress("first", "Checking output"), - _progress("second", "Running policy"), - _progress("first", "Checking output", active=False), - _progress("second", "Running policy", active=False), - ): - feedback.update_progress(update) - - assert statuses == ["Checking output", "Running policy", "Running policy", ""] diff --git a/libs/code/tests/unit_tests/hooks/test_manager.py b/libs/code/tests/unit_tests/hooks/test_manager.py new file mode 100644 index 00000000000..574a9061890 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_manager.py @@ -0,0 +1,94 @@ +"""Tests for `HooksManager` ownership of the shared presenter.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +from deepagents_code.approval_mode import ApprovalMode +from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager +from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +def _write_project_hooks(root: Path) -> Path: + (root / ".git").mkdir(parents=True) + hooks_dir = root / ".deepagents" + hooks_dir.mkdir() + (hooks_dir / "hooks.json").write_text( + json.dumps( + {"hooks": {"Stop": [{"hooks": [{"type": "command", "command": "true"}]}]}} + ), + encoding="utf-8", + ) + return root + + +def _isolate_hook_config(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + user_dir = tmp_path / "user" + user_dir.mkdir() + monkeypatch.setattr("deepagents_code.hooks.loading.DEFAULT_CONFIG_DIR", user_dir) + monkeypatch.setattr( + "deepagents_code.hooks.runtime.DEFAULT_CONFIG_DIR", tmp_path / "state" + ) + + +def _manager(cwd: Path, presenter: HookPresenter | None = None) -> HooksManager: + return HooksManager.create( + cwd=cwd, + identity=lambda: HookSessionIdentity("thread", ApprovalMode.MANUAL), + presenter=presenter, + ) + + +async def test_reload_keeps_one_presenter_shared_with_the_runtime( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Output sinks bound once must survive a working-directory change.""" + _isolate_hook_config(tmp_path, monkeypatch) + first = _write_project_hooks(tmp_path / "first") + second = _write_project_hooks(tmp_path / "second") + presenter = HookPresenter() + + manager = _manager(first, presenter) + assert manager.presenter is presenter + + await manager.reload(cwd=second) + + assert manager.presenter is presenter + assert _runtime_presenter(manager) is presenter + + +def test_attach_output_binds_sinks_and_replays_load_diagnostics( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A manager loaded before its UI must resurface what it could only log.""" + _isolate_hook_config(tmp_path, monkeypatch) + (tmp_path / "user" / "hooks.json").write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "command"}]}]}}), + encoding="utf-8", + ) + root = tmp_path / "project" + root.mkdir() + + manager = _manager(root) + notices: list[tuple[str, str]] = [] + + def record(message: str, severity: HookNoticeSeverity) -> None: + notices.append((message, severity)) + + manager.attach_output(notice=record) + + assert notices + assert all(severity in {"warning", "error"} for _, severity in notices) + + +def _runtime_presenter(manager: HooksManager) -> HookPresenter | None: + runtime = manager._runtime # asserting the shared-instance invariant + return runtime.presenter if runtime is not None else None diff --git a/libs/code/tests/unit_tests/hooks/test_presenter.py b/libs/code/tests/unit_tests/hooks/test_presenter.py new file mode 100644 index 00000000000..16843399fa9 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_presenter.py @@ -0,0 +1,80 @@ +"""Tests for the shared Hooks v2 presenter.""" + +from __future__ import annotations + +from deepagents_code.hooks.models.domain import HookEvent, PermissionEffect +from deepagents_code.hooks.presenter import ( + HookNoticeSeverity, + HookPresenter, + HookProgress, +) + + +def _progress( + operation_id: str, + message: str = "", + *, + active: bool = True, +) -> HookProgress: + return HookProgress( + operation_id=operation_id, + handler_id=f"Stop:{operation_id}", + event=HookEvent.STOP, + message=message, + active=active, + ) + + +def test_progress_keeps_latest_concurrent_status_visible() -> None: + statuses: list[str] = [] + + def record(message: str) -> None: + statuses.append(message) + + presenter = HookPresenter(status=record) + + for update in ( + _progress("first", "Checking output"), + _progress("second", "Running policy"), + _progress("first", "Checking output", active=False), + _progress("second", "Running policy", active=False), + ): + presenter.update_progress(update) + + assert statuses == ["Checking output", "Running policy", "Running policy", ""] + + +def test_progress_without_handler_message_falls_back_to_event_text() -> None: + statuses: list[str] = [] + + def record(message: str) -> None: + statuses.append(message) + + presenter = HookPresenter(status=record) + + presenter.update_progress(_progress("only")) + + assert statuses[0].startswith("Running Stop hook") + + +def test_attach_rebinds_sinks_on_the_same_presenter() -> None: + first: list[str] = [] + second: list[str] = [] + + def to_first(message: str, severity: HookNoticeSeverity) -> None: + del severity + first.append(message) + + def to_second(message: str, severity: HookNoticeSeverity) -> None: + del severity + second.append(message) + + presenter = HookPresenter(notice=to_first) + presenter.attach(notice=to_second) + presenter.present_permission( + "shell", + PermissionEffect(behavior="deny", reason="nope"), + ) + + assert first == [] + assert second == ["PermissionRequest hook denied shell: nope"] diff --git a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py index 199600c1d34..a2f80d61329 100644 --- a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py @@ -23,7 +23,6 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.client import fulfill_hook_invocation from deepagents_code.hooks.context import apply_hooks_context -from deepagents_code.hooks.feedback import HookFeedback from deepagents_code.hooks.interrupt import ( HOOK_INVOCATION_INTERRUPT_TYPE, build_hook_interrupt_payload, @@ -53,6 +52,7 @@ HookInvocationRequest, HookInvocationResponse, ) +from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.runtime import HooksRuntime from deepagents_code.hooks.server_middleware import ( ServerHooksMiddleware, @@ -885,7 +885,7 @@ async def test_fulfillment_is_idempotent_in_flight_and_after_completion( runtime = HooksRuntime.create( cwd=tmp_path, config_dir=config_dir, - feedback=HookFeedback( + presenter=HookPresenter( notice=lambda message, severity: notices.append((message, severity)) ), ) diff --git a/libs/code/tests/unit_tests/hooks/test_trust.py b/libs/code/tests/unit_tests/hooks/test_trust.py index 46f60e471f4..f762f6eb348 100644 --- a/libs/code/tests/unit_tests/hooks/test_trust.py +++ b/libs/code/tests/unit_tests/hooks/test_trust.py @@ -312,7 +312,6 @@ def _manager(cwd: Path, trust: WorkspaceTrust) -> HooksManager: return HooksManager.create( cwd=cwd, identity=lambda: HookSessionIdentity("thread", ApprovalMode.MANUAL), - notice=lambda _message: None, trust=trust, ) diff --git a/libs/code/tests/unit_tests/test_app.py b/libs/code/tests/unit_tests/test_app.py index ce7fec594c8..634e69e1fc5 100644 --- a/libs/code/tests/unit_tests/test_app.py +++ b/libs/code/tests/unit_tests/test_app.py @@ -13092,7 +13092,6 @@ async def test_resumed_history_populates_hook_transcript(self) -> None: app._session_state.hooks = HooksManager.adopting( runtime, identity=app._session_state.hook_identity, - notice=lambda _message: None, ) payload = _ThreadHistoryPayload( [], diff --git a/libs/code/tests/unit_tests/test_non_interactive.py b/libs/code/tests/unit_tests/test_non_interactive.py index c1cff7e681a..94fdb8a2de5 100644 --- a/libs/code/tests/unit_tests/test_non_interactive.py +++ b/libs/code/tests/unit_tests/test_non_interactive.py @@ -1415,7 +1415,6 @@ def _manager(runtime: MagicMock) -> HooksManager: thread_id="t1", approval_mode=ApprovalMode.MANUAL, ), - notice=lambda _message: None, ) @@ -1435,7 +1434,6 @@ async def test_headless_compact_permission_uses_live_context() -> None: approval_mode=approval_mode, prompt_id="00000000-0000-4000-8000-000000000001", ), - notice=lambda _message: None, ) state = StreamState(hooks=hooks) state.pending_interrupts["interrupt-1"] = { diff --git a/libs/code/tests/unit_tests/test_offload.py b/libs/code/tests/unit_tests/test_offload.py index cdb7fd3d963..2ef4cf8c4ea 100644 --- a/libs/code/tests/unit_tests/test_offload.py +++ b/libs/code/tests/unit_tests/test_offload.py @@ -1773,7 +1773,6 @@ async def _astream( # noqa: ANN202, RUF029 app._session_state.hooks = HooksManager.adopting( runtime, identity=app._session_state.hook_identity, - notice=lambda _message: None, ) app._agent = agent app._lc_thread_id = "test-thread" From 196ac068a83839192689005749a7b297168aae53 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 29 Jul 2026 13:37:05 -0700 Subject: [PATCH 5/6] cr --- libs/code/deepagents_code/app.py | 16 ++---- .../deepagents_code/client/non_interactive.py | 50 +++++++------------ libs/code/deepagents_code/hooks/engine.py | 16 +++--- libs/code/deepagents_code/hooks/manager.py | 36 +++++++------ .../tests/unit_tests/hooks/test_manager.py | 37 +++++++++++--- 5 files changed, 79 insertions(+), 76 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 3af137cb700..7384e262c23 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -605,7 +605,7 @@ class _ConfigWriteResult: from deepagents_code.hooks.models.domain import ( SessionStartCause, ) - from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter + from deepagents_code.hooks.presenter import HookNoticeSeverity from deepagents_code.hooks.trust import WorkspaceTrust from deepagents_code.mcp_tools import MCPServerInfo from deepagents_code.model_config import MissingProviderPackageError @@ -4367,14 +4367,6 @@ async def _post_paint_init(self) -> None: lambda: asyncio.create_task(self._run_session_start_sequence()), ) - def _create_hook_presenter(self) -> HookPresenter: - from deepagents_code.hooks.presenter import HookPresenter - - return HookPresenter( - notice=self._notify_hook_feedback, - status=self._update_hook_status, - ) - def _notify_hook_feedback( self, message: str, @@ -4422,7 +4414,8 @@ async def _init_session_state(self) -> None: session_state.hooks = HooksManager.create( cwd=Path(self._cwd), identity=session_state.hook_identity, - presenter=self._create_hook_presenter(), + notice=self._notify_hook_feedback, + status=self._update_hook_status, trust=self._hook_trust, ) # Re-read the app-owned selection last so a mode change during @@ -4443,7 +4436,8 @@ def _hooks(self) -> HooksManager: self._detached_hooks = HooksManager.adopting( None, identity=self._hook_identity, - presenter=self._create_hook_presenter(), + notice=self._notify_hook_feedback, + status=self._update_hook_status, ) return self._detached_hooks diff --git a/libs/code/deepagents_code/client/non_interactive.py b/libs/code/deepagents_code/client/non_interactive.py index 06c2eb42b3e..cd4714e34da 100644 --- a/libs/code/deepagents_code/client/non_interactive.py +++ b/libs/code/deepagents_code/client/non_interactive.py @@ -92,8 +92,8 @@ from deepagents_code.hooks.manager import HooksManager from deepagents_code.hooks.models.domain import SessionEndCause from deepagents_code.hooks.presenter import ( + HookNoticeCallback, HookNoticeSeverity, - HookPresenter, ) from deepagents_code.hooks.transcript import TranscriptRecorder @@ -212,7 +212,7 @@ def start(self, message: str = "Working...") -> None: """ if self._live is not None: return - renderable = self._renderable(message) + renderable = self._build_spinner(message) try: self._live = Live(renderable, console=self._console, transient=True) self._live.start() @@ -229,7 +229,7 @@ def update(self, message: str) -> None: if self._live is None: return try: - self._live.update(self._renderable(message)) + self._live.update(self._build_spinner(message)) except (AttributeError, TypeError, OSError) as exc: logger.warning("Spinner update failed: %s", exc) @@ -244,7 +244,7 @@ def stop(self) -> None: self._live = None @staticmethod - def _renderable(message: str) -> RichSpinner: + def _build_spinner(message: str) -> RichSpinner: return RichSpinner( "dots", text=Text(f" {message}", style="dim"), @@ -355,25 +355,24 @@ def _inert_hooks() -> HooksManager: return HooksManager.inert() -def _plain_hook_presenter(console: Console) -> HookPresenter: - """Build a presenter that prints notices plainly and shows no status. +def _plain_hook_notice(console: Console) -> HookNoticeCallback: + """Build a notice sink that prints hook output without styling. Used for hooks loaded before the run owns a spinner; `attach_output` later - rebinds this same presenter to the styled, spinner-aware sinks. + rebinds the manager's presenter to the styled, spinner-aware sinks. Args: console: Destination for notice text. Returns: - A presenter with an unstyled notice sink. + An unstyled notice sink. """ - from deepagents_code.hooks.presenter import HookPresenter def notice(message: str, severity: HookNoticeSeverity) -> None: del severity console.print(Text(message), highlight=False) - return HookPresenter(notice=notice) + return notice @dataclass @@ -1389,7 +1388,6 @@ async def _run_agent_loop( SessionEndCause, SessionStartCause, ) - from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.trust import WorkspaceTrust hook_owned_spinner = False @@ -1431,25 +1429,15 @@ def update_hook_status(message: str) -> None: approval_mode=resolved_approval_mode, prompt_id=prompt_id, ) - if hooks is not None: - state.hooks = hooks - state.hooks.attach_output(notice=present_hook_notice, status=hook_status) - else: - state.hooks = HooksManager.create( - cwd=Path.cwd(), - identity=lambda: identity, - presenter=HookPresenter( - notice=present_hook_notice, - status=hook_status, - ), - # Project hooks require an explicit opt-in, matching `--trust-project-mcp`. - # Persisted interactive trust deliberately does not carry into headless - # runs, so CI never inherits a grant made at someone's terminal. - trust=WorkspaceTrust.explicit_only( - Path.cwd(), - granted=trust_project_hooks, - ), - ) + state.hooks = hooks or HooksManager.create( + cwd=Path.cwd(), + identity=lambda: identity, + # Project hooks require an explicit opt-in, matching `--trust-project-mcp`. + # Persisted interactive trust deliberately does not carry into headless + # runs, so CI never inherits a grant made at someone's terminal. + trust=WorkspaceTrust.explicit_only(Path.cwd(), granted=trust_project_hooks), + ) + state.hooks.attach_output(notice=present_hook_notice, status=hook_status) state.hooks.apply_graph_context(context) context["approval_mode"] = resolved_approval_mode.value context["auto_approve"] = resolved_approval_mode is ApprovalMode.YOLO @@ -2055,7 +2043,7 @@ def discover_all_skills() -> tuple[list[ExtendedSkillMetadata], list[Path]]: identity=lambda: identity, # Plain output until `_run_agent_loop` attaches the styled, # spinner-aware sinks to this same presenter. - presenter=_plain_hook_presenter(console), + notice=_plain_hook_notice(console), # Explicit opt-in only; see `_run_agent_loop` for the rationale. trust=WorkspaceTrust.explicit_only(Path.cwd(), granted=trust_project_hooks), ) diff --git a/libs/code/deepagents_code/hooks/engine.py b/libs/code/deepagents_code/hooks/engine.py index 14f013d482a..84b0ff3f71e 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -50,6 +50,11 @@ async def run( are reduced in stable configuration order, independent of completion order. + The returned diagnostics are scoped to this invocation. Configuration + diagnostics collected while the snapshot loaded belong to whoever owns + the snapshot, which presents them once per load; repeating them here + would re-surface the same warning on every hook that runs. + Args: invocation: Native lifecycle invocation. transcript_path: Materialized client transcript path. @@ -75,11 +80,7 @@ async def run( return self.adapter.to_domain_decision( invocation, (), - diagnostics=( - *self.snapshot.diagnostics, - *match.diagnostics, - diagnostic, - ), + diagnostics=(*match.diagnostics, diagnostic), ) event = invocation.event.event @@ -105,10 +106,7 @@ async def run( return self.adapter.to_domain_decision( invocation, results, - diagnostics=( - *self.snapshot.diagnostics, - *match.diagnostics, - ), + diagnostics=match.diagnostics, ) diff --git a/libs/code/deepagents_code/hooks/manager.py b/libs/code/deepagents_code/hooks/manager.py index 2d96d3f1e36..33dc29e2c8c 100644 --- a/libs/code/deepagents_code/hooks/manager.py +++ b/libs/code/deepagents_code/hooks/manager.py @@ -128,7 +128,8 @@ def create( *, cwd: Path, identity: SessionIdentityProvider, - presenter: HookPresenter | None = None, + notice: HookNoticeCallback | None = None, + status: HookStatusCallback | None = None, trust: WorkspaceTrust | None = None, ) -> HooksManager: """Load hook configuration and return a ready manager. @@ -139,9 +140,9 @@ def create( Args: cwd: Session working directory used to resolve hook configuration. identity: Reads current thread, approval mode, and prompt id. - presenter: Presents notices, diagnostics, and progress. When - omitted, a sinkless one is created and output is only logged - until `attach_output` binds it to a UI. + notice: Sink for user-visible notices. When omitted, output is only + logged until `attach_output` binds a sink. + status: Sink for transient hook-owned status text. trust: Project-hook trust policy. Defaults to trusting nothing beyond what the persisted trust store already records. @@ -149,10 +150,10 @@ def create( A manager owning the loaded runtime, or an inert one on failure. """ policy = trust if trust is not None else WorkspaceTrust.none() - resolved = presenter if presenter is not None else HookPresenter() - runtime = _load_runtime(cwd, trust=policy, presenter=resolved) + presenter = HookPresenter(notice=notice, status=status) + runtime = _load_runtime(cwd, trust=policy, presenter=presenter) _present_load_diagnostics(runtime) - return cls(identity, resolved, runtime, policy) + return cls(identity, presenter, runtime, policy) @classmethod def adopting( @@ -160,31 +161,28 @@ def adopting( runtime: HooksRuntime | None, *, identity: SessionIdentityProvider, - presenter: HookPresenter | None = None, + notice: HookNoticeCallback | None = None, + status: HookStatusCallback | None = None, ) -> HooksManager: """Wrap an already-loaded runtime. A supplied runtime brought its own presenter, so that one is adopted - rather than displaced; `presenter`'s sinks are copied onto it to keep a - single instance. + rather than displaced; the given sinks are bound onto it so the runtime + and the manager keep sharing a single instance. Args: runtime: Preloaded runtime, or `None` when loading failed. identity: Reads current thread, approval mode, and prompt id. - presenter: Presents notices, diagnostics, and progress. + notice: Sink for user-visible notices. + status: Sink for transient hook-owned status text. Returns: A manager owning `runtime`. """ if runtime is None: - return cls( - identity, presenter if presenter is not None else HookPresenter() - ) - if presenter is not None: - runtime.presenter.attach( - notice=presenter.notice, - status=presenter.status, - ) + return cls(identity, HookPresenter(notice=notice, status=status)) + if notice is not None or status is not None: + runtime.presenter.attach(notice=notice, status=status) return cls(identity, runtime.presenter, runtime) @classmethod diff --git a/libs/code/tests/unit_tests/hooks/test_manager.py b/libs/code/tests/unit_tests/hooks/test_manager.py index 574a9061890..95bb08ff0b2 100644 --- a/libs/code/tests/unit_tests/hooks/test_manager.py +++ b/libs/code/tests/unit_tests/hooks/test_manager.py @@ -7,13 +7,15 @@ from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager -from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter +from deepagents_code.hooks.models.domain import PermissionEffect if TYPE_CHECKING: from pathlib import Path import pytest + from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter + def _write_project_hooks(root: Path) -> Path: (root / ".git").mkdir(parents=True) @@ -37,11 +39,10 @@ def _isolate_hook_config(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Non ) -def _manager(cwd: Path, presenter: HookPresenter | None = None) -> HooksManager: +def _manager(cwd: Path) -> HooksManager: return HooksManager.create( cwd=cwd, identity=lambda: HookSessionIdentity("thread", ApprovalMode.MANUAL), - presenter=presenter, ) @@ -53,10 +54,10 @@ async def test_reload_keeps_one_presenter_shared_with_the_runtime( _isolate_hook_config(tmp_path, monkeypatch) first = _write_project_hooks(tmp_path / "first") second = _write_project_hooks(tmp_path / "second") - presenter = HookPresenter() - manager = _manager(first, presenter) - assert manager.presenter is presenter + manager = _manager(first) + presenter = manager.presenter + assert _runtime_presenter(manager) is presenter await manager.reload(cwd=second) @@ -64,6 +65,30 @@ async def test_reload_keeps_one_presenter_shared_with_the_runtime( assert _runtime_presenter(manager) is presenter +def test_create_binds_sinks_to_the_manager_owned_presenter( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Callers pass sinks, never a presenter; the manager builds and owns it.""" + _isolate_hook_config(tmp_path, monkeypatch) + root = _write_project_hooks(tmp_path / "project") + notices: list[tuple[str, str]] = [] + + manager = HooksManager.create( + cwd=root, + identity=lambda: HookSessionIdentity("thread", ApprovalMode.MANUAL), + notice=lambda message, severity: notices.append((message, severity)), + ) + + assert _runtime_presenter(manager) is manager.presenter + manager.presenter.present_permission( + "shell", + PermissionEffect(behavior="deny", reason="nope"), + ) + + assert notices == [("PermissionRequest hook denied shell: nope", "warning")] + + def test_attach_output_binds_sinks_and_replays_load_diagnostics( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, From 5feffc510d40db285eef888fef267c529bb0af90 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Wed, 29 Jul 2026 13:48:49 -0700 Subject: [PATCH 6/6] test(code): enable experimental mode for HooksManager presenter tests CI merges main, which gates Hooks v2 behind DEEPAGENTS_CODE_EXPERIMENTAL; without the flag these tests saw an inert manager and failed. Co-authored-by: Cursor --- libs/code/tests/unit_tests/hooks/test_manager.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libs/code/tests/unit_tests/hooks/test_manager.py b/libs/code/tests/unit_tests/hooks/test_manager.py index 95bb08ff0b2..5faacf54848 100644 --- a/libs/code/tests/unit_tests/hooks/test_manager.py +++ b/libs/code/tests/unit_tests/hooks/test_manager.py @@ -5,6 +5,9 @@ import json from typing import TYPE_CHECKING +import pytest + +from deepagents_code._env_vars import EXPERIMENTAL from deepagents_code.approval_mode import ApprovalMode from deepagents_code.hooks.manager import HookSessionIdentity, HooksManager from deepagents_code.hooks.models.domain import PermissionEffect @@ -12,11 +15,15 @@ if TYPE_CHECKING: from pathlib import Path - import pytest - from deepagents_code.hooks.presenter import HookNoticeSeverity, HookPresenter +@pytest.fixture(autouse=True) +def _enable_hooks_v2(monkeypatch: pytest.MonkeyPatch) -> None: + """Hooks v2 only loads in experimental mode, which these tests exercise.""" + monkeypatch.setenv(EXPERIMENTAL, "1") + + def _write_project_hooks(root: Path) -> Path: (root / ".git").mkdir(parents=True) hooks_dir = root / ".deepagents"