feat(hooks): user-configurable lifecycle hooks (external command, 3-surface)#114
Conversation
…urface) Add a hook system that runs user-configured external commands at key points of the agent loop, mirroring the Claude Code / codex model (JSON over stdin/stdout, exit-code control, matcher scoping). Events: SessionStart, UserPromptSubmit, PreToolUse, PostToolUse, PostToolUseFailure, Stop. Design (see internal-doc/hooks-design.md): a transport-agnostic internal/hooks.Dispatcher is injected via context, so TUI, Web and ACP all share one implementation. Tool hooks mount as two eino middlewares around the approval gate (pre outside so PreToolUse can deny / rewrite args / pre-approve; post inside so it sees the real tool error). The runner's todo/goal auto-continuation loops merge with the Stop hook into a single continuation pipeline with one umbrella budget, one cancellation check, and stop_hook_active loop prevention. Security: only the user layer (~/.jcode/hooks.json) loads by default; project layers (.jcode/hooks.json[.local]) require JCODE_HOOKS_TRUST_PROJECT=1, since a hook is arbitrary code execution the moment its event fires — closing a clone-and-run RCE / approval-bypass vector. Trust-on-first-use is the follow-up. Matchers are exact-by-default (so "write" doesn't also hit todowrite); regex opts in via metacharacters. Fail-safe throughout: timeout/crash/bad exit never blocks the agent. Tested: 28 unit/integration tests (dispatcher, middleware, continuation, matcher, config); 2 real-LLM agent-eval cases; public docs at site/docs/overview/hooks.md with verified use-case snippets. Generated with Jack AI bot
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR introduces a new ChangesHooks feature
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Runner
participant PreHookMiddleware
participant Dispatcher
participant HookCommand
participant ApprovalMiddleware
participant Tool
participant PostHookMiddleware
Runner->>PreHookMiddleware: invoke tool call
PreHookMiddleware->>Dispatcher: Fire(PreToolUse, payload)
Dispatcher->>HookCommand: run configured hook via shell
HookCommand-->>Dispatcher: exit code + stdout/stderr
Dispatcher-->>PreHookMiddleware: Decision (deny/allow/updatedInput)
alt Decision denied
PreHookMiddleware-->>Runner: return deny message
else Decision allow/no-op
PreHookMiddleware->>ApprovalMiddleware: forward call (pre-approved flag optional)
ApprovalMiddleware->>Tool: execute tool
Tool-->>ApprovalMiddleware: result/error
ApprovalMiddleware->>PostHookMiddleware: pass result
PostHookMiddleware->>Dispatcher: Fire(PostToolUse/Failure, payload)
Dispatcher->>HookCommand: run configured hook
HookCommand-->>Dispatcher: exit code + stdout/stderr
Dispatcher-->>PostHookMiddleware: Decision (modifiedResult/context)
PostHookMiddleware-->>Runner: final result
end
sequenceDiagram
participant Runner
participant TodoGoalState
participant HooksDispatcher
participant StopHookCommand
Runner->>Runner: continuationSource(todo, goal, stopBlock)
alt todo incomplete
Runner->>TodoGoalState: continue lap (todo)
else goal active
Runner->>TodoGoalState: continue lap (goal)
else Stop hook configured
Runner->>HooksDispatcher: Fire(Stop, payload)
HooksDispatcher->>StopHookCommand: run Stop hook
StopHookCommand-->>HooksDispatcher: exit code / continue flag
HooksDispatcher-->>Runner: Decision (block/continue)
alt Decision blocks
Runner->>Runner: continue lap (stop_hook_active)
else Decision allows
Runner->>Runner: finish run
end
else no mechanism requests continuation
Runner->>Runner: finish run
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
exec.CommandContext SIGKILLs sh on deadline, but a grandchild it spawned keeps the stdout pipe open, so cmd.Wait blocks until the grandchild exits and defeats the timeout. WaitDelay force-closes the pipes so runHook returns promptly. Fixes TestFireTimeoutFailSafe on Linux CI. Generated with Jack AI bot
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
internal/hooks/config.go (1)
86-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefix the wrapped JSON error.
Keep wrapped errors tagged with the subsystem so callers that receive
loadFileerrors directly still get useful context.Proposed fix
- if err := json.Unmarshal(data, &c); err != nil { - return Config{}, fmt.Errorf("invalid JSON: %w", err) + if err := json.Unmarshal(data, &c); err != nil { + return Config{}, fmt.Errorf("hooks: invalid JSON: %w", err) }As per coding guidelines, use
fmt.Errorf("tool_name: %w", err)for wrapped errors in non-tool code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/hooks/config.go` around lines 86 - 89, The JSON unmarshal error in loadFile is wrapped without subsystem context; update the error returned from the json.Unmarshal path in Config loading to prefix the wrapped error with the tool/subsystem name while still using %w. Keep the fix localized to the loadFile logic in internal/hooks/config.go, so direct callers receive a tagged error like the rest of non-tool code.Source: Coding guidelines
internal/runner/continuation_test.go (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLocal const
capshadows the builtincap().Harmless here since the builtin isn't used in this scope, but the repo's linter config enables
revive/gocritic, some of which flag redefined predeclared identifiers. Trivial rename avoids the risk.🧹 Suggested rename
func TestContinuationSource(t *testing.T) { - const cap = 3 + const todoCapConst = 3 cases := []struct { name string todoIncomplete bool todoUsed int goalActive bool stopBlock bool want string }{ - {"todo wins over goal and stop", true, 0, true, true, "todo"}, - {"todo within cap", true, 2, false, false, "todo"}, - {"todo exhausted falls to goal", true, cap, true, false, "goal"}, - {"todo exhausted falls to stop", true, cap, false, true, "stop"}, + {"todo wins over goal and stop", true, 0, true, true, "todo"}, + {"todo within cap", true, 2, false, false, "todo"}, + {"todo exhausted falls to goal", true, todoCapConst, true, false, "goal"}, + {"todo exhausted falls to stop", true, todoCapConst, false, true, "stop"}, {"goal beats stop", false, 0, true, true, "goal"}, {"stop only", false, 0, false, true, "stop"}, {"nothing continues", false, 0, false, false, ""}, {"todo cap zero skips todo", true, 0, false, true, "stop"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - todoCap := cap + todoCap := todoCapConst🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/runner/continuation_test.go` at line 6, The local const named cap in continuation_test.go shadows the builtin cap(), which can trigger revive/gocritic lint warnings. Rename the constant in the relevant test scope to a non-predeclared identifier (for example, something test-specific) and update its uses in the surrounding continuation test setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal-doc/hooks-design.md`:
- Around line 80-84: The fenced code blocks in hooks-design.md are missing
language annotations, which triggers markdownlint MD040. Update the affected
fences in the documented sections, including the block around the hookMiddleware
diagram and the other referenced blocks, by adding the closest appropriate
language tag such as text so the markdown linter passes.
- Around line 217-220: The user-level hooks config path is incorrect in the
three-layer merge description. Update the hooks configuration list in
hooks-design.md so the user-level entry matches the implemented loader and PR
intent by using the `~/.jcode/hooks.json` path; keep the project and local
entries unchanged, and make sure the `hooks` terminology stays consistent with
the existing `.jcode` config layout.
In `@internal/command/acp.go`:
- Around line 673-675: The ACP prompt hook dispatcher is being attached too late
in the flow, so `SessionStart` and `UserPromptSubmit` cannot affect the prompt
before it is persisted. Move the
`hooks.NewSessionDispatcher`/`hooks.WithDispatcher` setup earlier in `acp.go`,
before `RecordUser` and `sess.history` updates, and make sure `SessionStart` is
fired once and `UserPromptSubmit` runs before any prompt recording. Use the
existing `promptCtx`, `sess`, and `config.Logger().Printf` wiring so hook
behavior matches the TUI path.
- Around line 673-675: ACP hook dispatcher setup should not assume a recorder
exists, because sess.rec is optional and sess.rec.UUID() can panic before
runner.Run. Update the hook metadata construction around hooks.WithDispatcher
and hooks.NewSessionDispatcher to guard access to the recorder, using a safe
fallback or skipping recorder-derived fields when sess.rec is nil, while
preserving the existing ACP hook behavior when a recorder is present.
In `@internal/command/interactive.go`:
- Around line 1007-1012: The hook dispatcher is being created before the resume
path restores the recorder UUID, so hook payloads and SessionStart can be bound
to the temporary session instead of the resumed one. In interactive.go, update
the flow around NewSessionDispatcher, WithDispatcher, and the SessionStart
dispatch so they run only after resume handling has called
st.rec.SetUUID(resumeUUID), or rebuild st.hookDisp and st.ctx immediately after
that UUID swap. Keep the dispatcher/session-start wiring aligned with the final
recorder UUID used by the resumed session.
In `@internal/hooks/dispatcher.go`:
- Around line 117-121: The hook dispatch path in dispatcher.go currently appends
and runs every HookSpec as a command, regardless of HookSpec.Type. Update the
filtering in the hook collection/execution flow around matchesTool so only specs
with type "command" are executed, and treat empty type only if you intend to
keep backward-compatible default behavior. For any unknown type, log a warning
and skip it before execution.
In `@internal/hooks/exec.go`:
- Around line 47-56: The hook execution in execHook is not fully torn down on
timeout and its stdout/stderr capture is unbounded. Update the
exec.CommandContext setup so the shell runs in its own process group and ensure
timeout cleanup kills the whole group, not just the parent shell. Replace the
plain bytes.Buffer capture in execHook with capped/truncating output handling
for both stdout and stderr, and keep the existing context timeout, cwd, and env
behavior intact.
In `@internal/hooks/hooks_test.go`:
- Around line 201-213: The timeout path in internal/hooks/exec.go is only
canceling the shell wrapper, so child processes can survive and keep the hook
running past the limit. Update the hook execution in the relevant exec helper to
terminate the full process tree on timeout, using process-group cleanup or
cmd.WaitDelay/custom cmd.Cancel, and keep the fail-safe allow behavior exercised
by TestFireTimeoutFailSafe.
In `@internal/web/server.go`:
- Around line 827-830: Move hook execution earlier in the web prompt flow so it
matches the TUI path: in the server handler around runner.Run, create and apply
the session dispatcher from hooks.NewSessionDispatcher before any user message
is recorded or appended to history. Ensure SessionStart runs once up front, then
invoke UserPromptSubmit before RecordUser and the history update, so hooks can
deny or modify the prompt before it reaches runner.Run.
In `@site/docs/overview/hooks.md`:
- Around line 46-56: The hooks overview omits the `PreCompact` and `PostCompact`
events even though `internal/hooks/types.go` exposes them and `Trigger` can
dispatch them. Update the events table and surrounding copy in the hooks docs to
mention all 8 events, adding entries for `PreCompact` and `PostCompact` with
their firing timing and typical use so `hooks.json` users can configure them
correctly.
---
Nitpick comments:
In `@internal/hooks/config.go`:
- Around line 86-89: The JSON unmarshal error in loadFile is wrapped without
subsystem context; update the error returned from the json.Unmarshal path in
Config loading to prefix the wrapped error with the tool/subsystem name while
still using %w. Keep the fix localized to the loadFile logic in
internal/hooks/config.go, so direct callers receive a tagged error like the rest
of non-tool code.
In `@internal/runner/continuation_test.go`:
- Line 6: The local const named cap in continuation_test.go shadows the builtin
cap(), which can trigger revive/gocritic lint warnings. Rename the constant in
the relevant test scope to a non-predeclared identifier (for example, something
test-specific) and update its uses in the surrounding continuation test setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0208931-0306-4d94-9fc8-0fe42a34dc56
📒 Files selected for processing (19)
agent-eval/suite/testcases.jsoninternal-doc/hooks-design.mdinternal/agent/agent.gointernal/agent/hook_middleware.gointernal/agent/hook_middleware_test.gointernal/command/acp.gointernal/command/interactive.gointernal/hooks/config.gointernal/hooks/context.gointernal/hooks/dispatcher.gointernal/hooks/exec.gointernal/hooks/hooks_test.gointernal/hooks/match.gointernal/hooks/types.gointernal/runner/approval.gointernal/runner/continuation_test.gointernal/runner/runner.gointernal/web/server.gosite/docs/overview/hooks.md
| // Inject the hook dispatcher so PreToolUse/PostToolUse/Stop hooks run on ACP | ||
| // too (parity with the TUI); reloaded per turn so hooks.json edits hot-apply. | ||
| promptCtx = hooks.WithDispatcher(promptCtx, hooks.NewSessionDispatcher(config.ConfigDir(), sess.env.Pwd(), sess.rec.UUID(), config.Logger().Printf)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Wire ACP prompt-level hooks before persisting the prompt.
This dispatcher is attached only after RecordUser and sess.history updates, so UserPromptSubmit cannot block/augment ACP prompts and SessionStart context is never injected. That leaves ACP able to bypass prompt hook policies enforced by the TUI. Create/reuse the dispatcher earlier, fire SessionStart once, and run UserPromptSubmit before recording/history updates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/command/acp.go` around lines 673 - 675, The ACP prompt hook
dispatcher is being attached too late in the flow, so `SessionStart` and
`UserPromptSubmit` cannot affect the prompt before it is persisted. Move the
`hooks.NewSessionDispatcher`/`hooks.WithDispatcher` setup earlier in `acp.go`,
before `RecordUser` and `sess.history` updates, and make sure `SessionStart` is
fired once and `UserPromptSubmit` runs before any prompt recording. Use the
existing `promptCtx`, `sess`, and `config.Logger().Printf` wiring so hook
behavior matches the TUI path.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Guard optional ACP recorders before building hook metadata.
Line 675 unconditionally calls sess.rec.UUID(), but this file treats the recorder as optional elsewhere. ACP sessions without a recorder will panic before runner.Run.
🐛 Proposed fix
- promptCtx = hooks.WithDispatcher(promptCtx, hooks.NewSessionDispatcher(config.ConfigDir(), sess.env.Pwd(), sess.rec.UUID(), config.Logger().Printf))
+ hookSessionID := string(params.SessionId)
+ if sess.rec != nil {
+ hookSessionID = sess.rec.UUID()
+ }
+ promptCtx = hooks.WithDispatcher(promptCtx, hooks.NewSessionDispatcher(config.ConfigDir(), sess.env.Pwd(), hookSessionID, config.Logger().Printf))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Inject the hook dispatcher so PreToolUse/PostToolUse/Stop hooks run on ACP | |
| // too (parity with the TUI); reloaded per turn so hooks.json edits hot-apply. | |
| promptCtx = hooks.WithDispatcher(promptCtx, hooks.NewSessionDispatcher(config.ConfigDir(), sess.env.Pwd(), sess.rec.UUID(), config.Logger().Printf)) | |
| // Inject the hook dispatcher so PreToolUse/PostToolUse/Stop hooks run on ACP | |
| // too (parity with the TUI); reloaded per turn so hooks.json edits hot-apply. | |
| hookSessionID := string(params.SessionId) | |
| if sess.rec != nil { | |
| hookSessionID = sess.rec.UUID() | |
| } | |
| promptCtx = hooks.WithDispatcher(promptCtx, hooks.NewSessionDispatcher(config.ConfigDir(), sess.env.Pwd(), hookSessionID, config.Logger().Printf)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/command/acp.go` around lines 673 - 675, ACP hook dispatcher setup
should not assume a recorder exists, because sess.rec is optional and
sess.rec.UUID() can panic before runner.Run. Update the hook metadata
construction around hooks.WithDispatcher and hooks.NewSessionDispatcher to guard
access to the recorder, using a safe fallback or skipping recorder-derived
fields when sess.rec is nil, while preserving the existing ACP hook behavior
when a recorder is present.
✅ Confirmed as addressed by @cnjack
There was a problem hiding this comment.
Fixed the panic — guard sess.rec and fall back to params.SessionId. The prompt-level part (UserPromptSubmit/SessionStart on ACP) is deferred to a documented follow-up (design §11): tool + Stop hooks already run on ACP, and project hooks are off by default, so it is an incompleteness rather than an escalation.
🤖 Addressed by Claude Code
There was a problem hiding this comment.
@cnjack, sounds good — the guard for sess.rec with fallback to params.SessionId matches what's now in the file, so that panic risk is resolved.
On the deferral of UserPromptSubmit/SessionStart wiring for ACP: that's a reasonable call given tool + Stop hooks already run on ACP and project hooks are off by default. Documenting it as a known gap in the design doc is a good way to track it so it doesn't get lost. Thanks for clarifying the scope.
| // Inject the hook dispatcher so PreToolUse/PostToolUse/Stop hooks run on the | ||
| // Web surface too (parity with the TUI); reloaded per turn for hot-apply. | ||
| hookCtx := hooks.WithDispatcher(runCtx, hooks.NewSessionDispatcher(config.ConfigDir(), eng.env.Pwd(), recorder.UUID(), config.Logger().Printf)) | ||
| resp := runner.Run(hookCtx, agent, history, eng.eventHandler, recorder, eng.todoStore, eng.env.GoalStore, s.tracer, eng.tokenUsage) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Run web prompt hooks before recording/history updates.
Line 829 only makes runner-level hooks available after the web path has already emitted/recorded the user message and appended history. UserPromptSubmit cannot deny/augment web prompts, and SessionStart context is never applied, so web bypasses prompt hook policies unlike the TUI. Build/fire the dispatcher earlier, apply SessionStart once, and execute UserPromptSubmit before RecordUser and history append.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/web/server.go` around lines 827 - 830, Move hook execution earlier
in the web prompt flow so it matches the TUI path: in the server handler around
runner.Run, create and apply the session dispatcher from
hooks.NewSessionDispatcher before any user message is recorded or appended to
history. Ensure SessionStart runs once up front, then invoke UserPromptSubmit
before RecordUser and the history update, so hooks can deny or modify the prompt
before it reaches runner.Run.
There was a problem hiding this comment.
Deferred (documented in design §11): tool + Stop hooks already run on Web (dispatcher injected into the run ctx). Wiring UserPromptSubmit/SessionStart on Web/ACP needs surface-specific deny semantics + a per-session SessionStart lifecycle. Project hooks are off by default (only the user own ~/.jcode/hooks.json), so this is an incompleteness, not a policy bypass.
🤖 Addressed by Claude Code
There was a problem hiding this comment.
@cnjack, that's a fair distinction — thanks for the clarification. Tool-level and Stop hooks already provide the main safety/observability surface on Web, and gating UserPromptSubmit/SessionStart behind proper per-session lifecycle + deny semantics rather than bolting it on ad hoc is the right call. Since project-level hooks require explicit opt-in via JCODE_HOOKS_TRUST_PROJECT=1, the exposure here is limited to a user's own trusted ~/.jcode/hooks.json, so treating this as a documented incompleteness (tracked in design §11) rather than a blocking security gap makes sense. No action needed for this PR.
…ening) acp: guard optional recorder before sess.rec.UUID() (panic). interactive: rebuild dispatcher + fire SessionStart after resume restores the UUID, gate the setup SessionStart to fresh sessions (avoid double-fire and wrong session_id). dispatcher: skip unsupported hook types. exec: cap stdout/stderr at 1 MiB and run hooks in their own process group (unix) so a timeout kills the whole tree, not just sh. types: drop unwired PreCompact/PostCompact/Trigger so code and docs agree. Plus doc path fix, error prefix, and const rename nits. prompt-level hooks (UserPromptSubmit/SessionStart) remain TUI-only in v1; ACP/Web parity is a documented follow-up. Generated with Jack AI bot
What
A user-configurable hook system: run your own commands at key points in the agent loop — before/after a tool runs, on prompt submit, on session start, and when the agent is about to stop. Same model as Claude Code / codex (JSON over stdin/stdout, exit-code control, matcher scoping).
Events:
SessionStart,UserPromptSubmit,PreToolUse,PostToolUse,PostToolUseFailure,Stop.Why / design
Full design in
internal-doc/hooks-design.md. Key choices:internal/hooks.Dispatcheris injected viacontext, so TUI, Web and ACP all share a single implementation instead of wiring hooks per surface.presits outside approval (soPreToolUsecan deny, rewrite args, or pre-approve to skip the prompt);postsits inside (so it sees the real tool error and can rewrite the result).Stophook into one pipeline with a single umbrella budget, one cancellation check, andstop_hook_activeloop-prevention — instead of three independent loops that could compound or bypass each other.Security
A hook is arbitrary code execution the instant its event fires (
SessionStartruns on startup). So only~/.jcode/hooks.json(user layer) loads by default; project layers (.jcode/hooks.json[.local]) requireJCODE_HOOKS_TRUST_PROJECT=1. This closes a clone-and-run RCE / approval-bypass vector. Trust-on-first-use (per-hook hash confirmation) is the planned follow-up.Notes for reviewers
"write"does not also matchtodowrite); regex opts in via metacharacters ("mcp__.*"). jcode tool names plus Claude Code aliases (Bash/Write/…) both match.Testing
agent-evalcases (hook_pretooluse_deny,hook_posttooluse_side_effect).site/docs/overview/hooks.mdwith 8 use-case snippets, each verified against sample payloads.Not in this PR (follow-ups)
Trust-on-first-use UI,
/hooksmanagement panel,Pre/PostCompactevents, surfacing hookSystemMessagein the UI, teammate/subagent hook coverage.Generated with Jack AI bot
Summary by CodeRabbit
New Features
Documentation
Tests