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 d902a2ae484..9ed71f2f9ad 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -605,6 +605,7 @@ class _ConfigWriteResult: from deepagents_code.hooks.models.domain import ( SessionStartCause, ) + 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 @@ -2282,7 +2283,6 @@ def __init__( self.hooks: HooksManager = HooksManager.adopting( None, identity=self.hook_identity, - notice=lambda _message: None, ) """Client-side Hooks v2 coordinator. @@ -4367,6 +4367,18 @@ async def _post_paint_init(self) -> None: lambda: asyncio.create_task(self._run_session_start_sequence()), ) + def _notify_hook_feedback( + self, + message: str, + severity: HookNoticeSeverity, + ) -> 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. @@ -4402,7 +4414,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), + 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 @@ -4423,7 +4436,8 @@ def _hooks(self) -> HooksManager: self._detached_hooks = HooksManager.adopting( None, identity=self._hook_identity, - notice=lambda message: self.notify(message, markup=False), + 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 7d66981d7a6..cd4714e34da 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 ( + HookNoticeCallback, + HookNoticeSeverity, + ) from deepagents_code.hooks.transcript import TranscriptRecorder logger = logging.getLogger(__name__) @@ -192,6 +196,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 +212,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._build_spinner(message) try: self._live = Live(renderable, console=self._console, transient=True) self._live.start() @@ -215,6 +220,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._build_spinner(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 +243,14 @@ def stop(self) -> None: finally: self._live = None + @staticmethod + def _build_spinner(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. @@ -329,6 +355,26 @@ def _inert_hooks() -> HooksManager: return HooksManager.inert() +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 the manager's presenter to the styled, spinner-aware sinks. + + Args: + console: Destination for notice text. + + Returns: + An unstyled notice sink. + """ + + def notice(message: str, severity: HookNoticeSeverity) -> None: + del severity + console.print(Text(message), highlight=False) + + return notice + + @dataclass class StreamState: """Mutable state accumulated while iterating over the agent stream.""" @@ -1344,6 +1390,38 @@ async def _run_agent_loop( ) from deepagents_code.hooks.trust import WorkspaceTrust + hook_owned_spinner = False + + def present_hook_notice( + message: str, + severity: HookNoticeSeverity, + ) -> 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...") + + 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( @@ -1354,12 +1432,12 @@ async def _run_agent_loop( 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), ) + 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 @@ -1963,7 +2041,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. + 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/client.py b/libs/code/deepagents_code/hooks/client.py index f4997c0bdde..eb775d3a586 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.presenter.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..4f6fe39fa65 100644 --- a/libs/code/deepagents_code/hooks/client_lifecycle.py +++ b/libs/code/deepagents_code/hooks/client_lifecycle.py @@ -2,8 +2,6 @@ from __future__ import annotations -import logging -import sys from dataclasses import dataclass, field from typing import TYPE_CHECKING from uuid import UUID @@ -15,7 +13,6 @@ DcodeNotificationKind, HookContext, HookDecision, - HookDiagnostic, HookDomainEvent, HookEvent, HookInvocation, @@ -36,24 +33,29 @@ UserPromptSubmitDecision, UserPromptSubmitEvent, ) +from deepagents_code.hooks.permissions import ( + PermissionHookOutcome, + permission_hook_outcome, +) 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 presenter(self) -> HookPresenter: ... + 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.""" @@ -105,10 +107,14 @@ 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 # 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) @@ -279,6 +285,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 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 +385,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.runtime.presenter.present_permission(tool_name, permission) + async def _invoke( self, context: ClientHookContext, @@ -360,31 +413,5 @@ async def _invoke( event=event, ) decision = await self.runtime.invoke(invocation) - self._apply_common_effects(decision) + self.runtime.presenter.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..84b0ff3f71e 100644 --- a/libs/code/deepagents_code/hooks/engine.py +++ b/libs/code/deepagents_code/hooks/engine.py @@ -3,22 +3,28 @@ 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.models.domain import HookDiagnostic +from deepagents_code.hooks.presenter import HookProgress 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__) @dataclass(frozen=True, slots=True) @@ -36,6 +42,7 @@ async def run( *, transcript_path: Path, agent_transcript_path: Path | None = None, + on_progress: Callable[[HookProgress], None] | None = None, ) -> HookDecision: """Execute matching handlers and return a normalized decision. @@ -43,10 +50,16 @@ 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. agent_transcript_path: Materialized subagent transcript path. + on_progress: Optional handler lifecycle callback. Returns: The event-specific decision produced by ordered hook reduction. @@ -67,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 @@ -82,12 +91,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}", + on_progress=on_progress, ) for handler in match.handlers ) @@ -95,8 +106,59 @@ async def run( return self.adapter.to_domain_decision( invocation, results, - diagnostics=( - *self.snapshot.diagnostics, - *match.diagnostics, + diagnostics=match.diagnostics, + ) + + +async def _run_handler( + handler: HookHandler, + payload: bytes, + *, + cwd: Path, + default_timeout: float, + max_output_bytes: int, + operation_id: str, + on_progress: Callable[[HookProgress], None] | None, +) -> HandlerResult: + message = (handler.status_message or "").strip() + _report_progress( + on_progress, + HookProgress( + operation_id=operation_id, + handler_id=handler.id, + event=handler.event, + message=message, + active=True, + ), + ) + try: + return await run_command_handler( + handler, + payload, + cwd=cwd, + default_timeout=default_timeout, + max_output_bytes=max_output_bytes, + ) + finally: + _report_progress( + on_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/manager.py b/libs/code/deepagents_code/hooks/manager.py index 7ecae012c75..426de4932eb 100644 --- a/libs/code/deepagents_code/hooks/manager.py +++ b/libs/code/deepagents_code/hooks/manager.py @@ -25,8 +25,8 @@ from deepagents_code.hooks.permissions import ( PermissionHookOutcome, PermissionPlan, - permission_hook_outcome, ) +from deepagents_code.hooks.presenter import HookPresenter from deepagents_code.hooks.trust import WorkspaceTrust if TYPE_CHECKING: @@ -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,7 +128,8 @@ def create( *, cwd: Path, identity: SessionIdentityProvider, - notice: Callable[[str], None], + notice: HookNoticeCallback | None = None, + status: HookStatusCallback | None = None, trust: WorkspaceTrust | None = None, ) -> HooksManager: """Load hook configuration and return a ready manager. @@ -134,7 +144,9 @@ 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. + 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. @@ -142,7 +154,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() - return cls(identity, notice, _load_runtime(cwd, trust=policy), policy) + presenter = HookPresenter(notice=notice, status=status) + runtime = _load_runtime(cwd, trust=policy, presenter=presenter) + _present_load_diagnostics(runtime) + return cls(identity, presenter, runtime, policy) @classmethod def adopting( @@ -150,19 +165,29 @@ def adopting( runtime: HooksRuntime | None, *, identity: SessionIdentityProvider, - notice: Callable[[str], 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; 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. - notice: Surfaces hook `systemMessage` notices to the user. + notice: Sink for user-visible notices. + status: Sink for transient hook-owned status text. Returns: A manager owning `runtime`. """ - return cls(identity, notice, runtime) + if runtime is None: + 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 def inert(cls) -> HooksManager: @@ -173,11 +198,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_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: + notice: Sink for user-visible notices. + status: Sink for transient hook-owned status text. + """ + self.presenter.attach(notice=notice, status=status) + _present_load_diagnostics(self._runtime) @property def enabled(self) -> bool: @@ -201,7 +241,8 @@ async def reload(self, *, cwd: Path) -> None: 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. @@ -215,8 +256,10 @@ async def reload(self, *, cwd: Path) -> None: _load_runtime, cwd, trust=self.trust, + presenter=self.presenter, ) self._service = self._build_service() + _present_load_diagnostics(self._runtime) async def on_session_start( self, @@ -362,7 +405,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", @@ -370,7 +413,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( @@ -525,7 +568,7 @@ def _build_service(self) -> ClientHookService | None: runtime = self._runtime if runtime is None: return None - return ClientHookService(runtime, notice=self.notice) + return ClientHookService(runtime) def _context(self, *, thread_id: str | None = None) -> ClientHookContext: identity = self.identity() @@ -536,7 +579,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.presenter.present_diagnostics(runtime.snapshot.diagnostics) + + +def _load_runtime( + cwd: Path, + *, + trust: WorkspaceTrust, + presenter: HookPresenter, +) -> 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 @@ -545,6 +604,7 @@ def _load_runtime(cwd: Path, *, trust: WorkspaceTrust) -> HooksRuntime | None: Args: cwd: Session working directory. trust: Policy deciding whether project hooks may load. + presenter: The manager's presenter, shared with the new runtime. Returns: The loaded runtime, `None` when configuration could not be loaded, and @@ -556,7 +616,11 @@ def _load_runtime(cwd: Path, *, trust: WorkspaceTrust) -> HooksRuntime | None: if not is_env_truthy(EXPERIMENTAL): return None try: - return HooksRuntime.create(cwd=cwd, workspace_trusted=trust.allows(cwd)) + return HooksRuntime.create( + cwd=cwd, + workspace_trusted=trust.allows(cwd), + presenter=presenter, + ) except Exception: logger.exception("Failed to load hook configuration; hooks disabled") return None diff --git a/libs/code/deepagents_code/hooks/presenter.py b/libs/code/deepagents_code/hooks/presenter.py new file mode 100644 index 00000000000..8c78ca29ca4 --- /dev/null +++ b/libs/code/deepagents_code/hooks/presenter.py @@ -0,0 +1,217 @@ +"""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 + +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__) + +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: HookNoticeSeverity) -> 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 + active: bool + message: str = "" + """Handler-authored status text. Empty when the handler supplied none.""" + + +@dataclass(slots=True) +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. + + 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: HookNoticeSeverity = ( + "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] = _status_text(progress) + 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: HookNoticeSeverity) -> bool: + if self.notice is None: + logger.warning("Hook notice (no sink attached): %s", message) + return True + try: + self.notice(message, severity) + except Exception: + logger.warning("Failed to surface hook notice", 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 _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": + 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/runtime.py b/libs/code/deepagents_code/hooks/runtime.py index 7c5abe22c7c..6d76a105a70 100644 --- a/libs/code/deepagents_code/hooks/runtime.py +++ b/libs/code/deepagents_code/hooks/runtime.py @@ -18,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 @@ -62,6 +63,7 @@ class HooksRuntime: """ project_hooks_loaded: bool + presenter: HookPresenter fulfillments: HookFulfillmentLedger @classmethod @@ -72,6 +74,7 @@ def create( workspace_trusted: bool = False, config_dir: Path | None = None, transcript_root: Path | None = None, + presenter: HookPresenter | None = None, ) -> HooksRuntime: """Load configuration once and freeze a session runtime. @@ -85,6 +88,8 @@ def create( Defaults to `~/.deepagents/transcripts` regardless of `config_dir` (project and test hook configs must not relocate the global transcript store). + 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. @@ -114,6 +119,7 @@ def create( cwd=project_context.user_cwd, workspace_trusted=workspace_trusted, project_hooks_loaded=loaded.project_source_loaded, + presenter=presenter if presenter is not None else HookPresenter(), fulfillments=HookFulfillmentLedger(), ) @@ -176,6 +182,7 @@ async def invoke(self, invocation: HookInvocation) -> HookDecision: prepared.invocation, transcript_path=prepared.transcript_path, agent_transcript_path=prepared.agent_transcript_path, + on_progress=self.presenter.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/test_client_lifecycle.py b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py index dfdd018b5c3..44b64b2ee75 100644 --- a/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_client_lifecycle.py @@ -30,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 @@ -40,6 +41,7 @@ class _Runtime: cwd: Path decisions: deque[HookDecision] invocations: list[HookInvocation] = field(default_factory=list) + presenter: HookPresenter = field(default_factory=HookPresenter) def configured_events(self) -> frozenset[HookEvent]: return frozenset(decision.event for decision in self.decisions) @@ -76,13 +78,20 @@ async def test_common_effects_context_and_live_hook_fields( severity="warning", message="diagnostic", ) - ], + ] + * 2, ) ] ), ) 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 @@ -90,7 +99,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..c7cf764f72f 100644 --- a/libs/code/tests/unit_tests/hooks/test_engine.py +++ b/libs/code/tests/unit_tests/hooks/test_engine.py @@ -63,6 +63,7 @@ from pathlib import Path from deepagents_code.hooks.models.domain import HookDomainEvent + from deepagents_code.hooks.presenter import HookProgress from deepagents_code.json_types import JsonObject @@ -1624,6 +1625,45 @@ 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", + } + ] + } + ] + } + ) + ) + progress: list[HookProgress] = [] + + await HookEngine(snapshot).run( + _invocation( + tmp_path, + SessionStartEvent( + event=HookEvent.SESSION_START, + cause=SessionStartCause.STARTUP, + ), + ), + transcript_path=_transcript_path(tmp_path), + on_progress=progress.append, + ) + + 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: original = _config( { 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..5faacf54848 --- /dev/null +++ b/libs/code/tests/unit_tests/hooks/test_manager.py @@ -0,0 +1,126 @@ +"""Tests for `HooksManager` ownership of the shared presenter.""" + +from __future__ import annotations + +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 + +if TYPE_CHECKING: + from pathlib import Path + + 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" + 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) -> HooksManager: + return HooksManager.create( + cwd=cwd, + identity=lambda: HookSessionIdentity("thread", ApprovalMode.MANUAL), + ) + + +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") + + manager = _manager(first) + presenter = manager.presenter + assert _runtime_presenter(manager) is presenter + + await manager.reload(cwd=second) + + assert manager.presenter is presenter + 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, +) -> 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 38299df72a9..0a929ea6389 100644 --- a/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py +++ b/libs/code/tests/unit_tests/hooks/test_server_lifecycle.py @@ -52,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, @@ -854,7 +855,6 @@ async def test_fulfill_hook_invocation_runs_engine(tmp_path: Path) -> None: async def test_fulfillment_is_idempotent_in_flight_and_after_completion( tmp_path: Path, - caplog: pytest.LogCaptureFixture, ) -> None: config_dir = tmp_path / "config" config_dir.mkdir() @@ -886,21 +886,25 @@ 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]] = [] + runtime = HooksRuntime.create( + cwd=tmp_path, + config_dir=config_dir, + presenter=HookPresenter( + 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 == [("once", "information")] def test_snapshot_configured_server_events() -> None: diff --git a/libs/code/tests/unit_tests/hooks/test_trust.py b/libs/code/tests/unit_tests/hooks/test_trust.py index 6541411f83e..9be4ef19b72 100644 --- a/libs/code/tests/unit_tests/hooks/test_trust.py +++ b/libs/code/tests/unit_tests/hooks/test_trust.py @@ -319,7 +319,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 e3f1676a57e..d02215a1a28 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 2c4ccc01efe..f031fc8084c 100644 --- a/libs/code/tests/unit_tests/test_non_interactive.py +++ b/libs/code/tests/unit_tests/test_non_interactive.py @@ -1419,7 +1419,6 @@ def _manager(runtime: MagicMock) -> HooksManager: thread_id="t1", approval_mode=ApprovalMode.MANUAL, ), - notice=lambda _message: None, ) @@ -1439,7 +1438,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" 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..975bf818b8b 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,22 @@ 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") + 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 [bold]hook[/bold]" + + bar.set_status_message("", source="hooks") + await pilot.pause() + assert str(msg.render()) == "Still loading" + 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: