Skip to content

feat(hooks): refactor for extensibility, add 5 events and 3 builtins#2519

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/improving-hooks-design-for-extensibility-76cf250c
Apr 27, 2026
Merged

feat(hooks): refactor for extensibility, add 5 events and 3 builtins#2519
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/improving-hooks-design-for-extensibility-76cf250c

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Apr 27, 2026

Reshape pkg/hooks from "7 hard-coded events with bespoke per-event
boilerplate" into a generic Dispatch/Has surface backed by a pluggable
Registry. Add 5 new lifecycle events and 3 in-process builtin hooks
that extract previously-inline runtime concerns. Strictly additive at
the YAML layer: every existing hooks YAML keeps parsing untouched.

Architecture:

  • Handler/Registry seam: Executor resolves each Hook's HookType
    against a Registry of HandlerFactory values. The default registry
    ships HookTypeCommand (the long-standing shell-out behavior, lifted
    into a Handler) and HookTypeBuiltin (a new handler kind that
    dispatches by name to an in-process Go function). Adding a new
    handler kind is one Register call; extending Hook with args: []
    lets builtins receive per-hook parameters.
  • Dispatch/Has API: collapse 7 ExecuteXxx + 7 HasXxxHooks methods
    into Dispatch(ctx, event, input) and Has(event), backed by an
    eventTable built once at construction. Adding a new event becomes
    a one-line entry; the executor needs no per-event methods or fields.
  • Builtin lookup is per-Registry, not global: NewLocalRuntime builds
    a private hooks.Registry, registers the three runtime-owned
    builtins on it, and threads it into every executor. No init()
    functions, no shared mutable state, no cross-runtime pollution.

New events (5):

turn_start fires per model call, AFTER the persisted
messages are read. AdditionalContext is appended
transiently and never persisted, so per-turn
signals refresh every turn.
before_llm_call just before each model invocation. Observational.
after_llm_call just after a successful model call. Carries the
assistant text via stop_response.
on_error structured handler for runtime errors (model
failures, tool-call loops). Fires alongside the
generic notification event.
on_max_iterations structured handler for hitting max_iterations.
Fires alongside the generic notification event.

Extracted runtime concerns (3 builtins):

add_date turn_start, "Today's date: YYYY-MM-DD"
add_environment_info session_start, working dir / OS / arch / git
add_prompt_files turn_start, reads files via Hook.Args

The runtime auto-injects them based on the existing AddDate /
AddEnvironmentInfo / AddPromptFiles agent flags, so YAMLs with those
flags keep working with identical effective behavior. Users can also
write the builtins explicitly in YAML now that the schema exposes
type=builtin.

Schema additions in pkg/config/latest:

  • HooksConfig gains turn_start, before_llm_call, after_llm_call,
    on_error, on_max_iterations top-level keys.
  • HookDefinition gains args: [string], used by builtin handlers to
    receive per-hook parameters.
  • HookDefinition.type accepts "builtin" alongside "command".
  • agent-schema.json updated to match.

User-facing surface:

  • examples/hooks.yaml is a single canonical example covering all
    twelve events and both handler kinds (replaces five separate
    hooks_*.yaml files).
  • The legacy ExecuteXxx / HasXxxHooks shims on Executor are gone;
    every caller migrated to Dispatch / Has.

Bug fixes uncovered along the way:

  • Default LocalRuntime.workingDir to os.Getwd() when no caller passed
    WithWorkingDir, matching the session's default. Fixes
    add_prompt_files reading an empty Cwd in CLI invocations.
  • Restore cache_control marker on the last extra in
    Session.GetMessages so AddPromptFiles content keeps participating
    in Anthropic prompt caching.
  • Route plain stdout from turn_start command hooks into
    AdditionalContext (was silently dropped, while session_start did
    surface it).
  • Extend hook deduplication from (type, command) to (type, command,
    args) so two add_prompt_files hooks with different file lists fire
    as distinct invocations.
  • Fix MergeHooks to also merge Stop, Notification, and the new event
    slices (previously dropped Stop / Notification when both base and
    CLI were non-empty).
  • Align hook helpers' log levels to Warn
    (executeSessionEndHooks was the odd Error out).

Validated with go vet ./..., golangci-lint run ./... (0 issues), and
go test -race across pkg/hooks, pkg/runtime, pkg/session, pkg/config
(all versions), and e2e.

Assisted-By: docker-agent

Reshape pkg/hooks from "7 hard-coded events with bespoke per-event
boilerplate" into a generic Dispatch/Has surface backed by a pluggable
Registry. Add 5 new lifecycle events and 3 in-process builtin hooks
that extract previously-inline runtime concerns. Strictly additive at
the YAML layer: every existing hooks YAML keeps parsing untouched.

Architecture:

- Handler/Registry seam: Executor resolves each Hook's HookType
  against a Registry of HandlerFactory values. The default registry
  ships HookTypeCommand (the long-standing shell-out behavior, lifted
  into a Handler) and HookTypeBuiltin (a new handler kind that
  dispatches by name to an in-process Go function). Adding a new
  handler kind is one Register call; extending Hook with `args: []`
  lets builtins receive per-hook parameters.
- Dispatch/Has API: collapse 7 ExecuteXxx + 7 HasXxxHooks methods
  into Dispatch(ctx, event, input) and Has(event), backed by an
  eventTable built once at construction. Adding a new event becomes
  a one-line entry; the executor needs no per-event methods or fields.
- Builtin lookup is per-Registry, not global: NewLocalRuntime builds
  a private hooks.Registry, registers the three runtime-owned
  builtins on it, and threads it into every executor. No init()
  functions, no shared mutable state, no cross-runtime pollution.

New events (5):

  turn_start         fires per model call, AFTER the persisted
                     messages are read. AdditionalContext is appended
                     transiently and never persisted, so per-turn
                     signals refresh every turn.
  before_llm_call    just before each model invocation. Observational.
  after_llm_call     just after a successful model call. Carries the
                     assistant text via stop_response.
  on_error           structured handler for runtime errors (model
                     failures, tool-call loops). Fires alongside the
                     generic notification event.
  on_max_iterations  structured handler for hitting max_iterations.
                     Fires alongside the generic notification event.

Extracted runtime concerns (3 builtins):

  add_date              turn_start, "Today's date: YYYY-MM-DD"
  add_environment_info  session_start, working dir / OS / arch / git
  add_prompt_files      turn_start, reads files via Hook.Args

The runtime auto-injects them based on the existing AddDate /
AddEnvironmentInfo / AddPromptFiles agent flags, so YAMLs with those
flags keep working with identical effective behavior. Users can also
write the builtins explicitly in YAML now that the schema exposes
type=builtin.

Schema additions in pkg/config/latest:

- HooksConfig gains turn_start, before_llm_call, after_llm_call,
  on_error, on_max_iterations top-level keys.
- HookDefinition gains args: [string], used by builtin handlers to
  receive per-hook parameters.
- HookDefinition.type accepts "builtin" alongside "command".
- agent-schema.json updated to match.

User-facing surface:

- examples/hooks.yaml is a single canonical example covering all
  twelve events and both handler kinds (replaces five separate
  hooks_*.yaml files).
- The legacy ExecuteXxx / HasXxxHooks shims on Executor are gone;
  every caller migrated to Dispatch / Has.

Bug fixes uncovered along the way:

- Default LocalRuntime.workingDir to os.Getwd() when no caller passed
  WithWorkingDir, matching the session's default. Fixes
  add_prompt_files reading an empty Cwd in CLI invocations.
- Restore cache_control marker on the last extra in
  Session.GetMessages so AddPromptFiles content keeps participating
  in Anthropic prompt caching.
- Route plain stdout from turn_start command hooks into
  AdditionalContext (was silently dropped, while session_start did
  surface it).
- Extend hook deduplication from (type, command) to (type, command,
  args) so two add_prompt_files hooks with different file lists fire
  as distinct invocations.
- Fix MergeHooks to also merge Stop, Notification, and the new event
  slices (previously dropped Stop / Notification when both base and
  CLI were non-empty).
- Align hook helpers' log levels to Warn
  (executeSessionEndHooks was the odd Error out).

Validated with go vet ./..., golangci-lint run ./... (0 issues), and
go test -race across pkg/hooks, pkg/runtime, pkg/session, pkg/config
(all versions), and e2e.

Assisted-By: docker-agent
@dgageot dgageot requested a review from a team as a code owner April 27, 2026 08:50
@dgageot dgageot merged commit 514714b into docker:main Apr 27, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants