Skip to content

refactor(runtime): consolidate hook orchestration and cache executors#2523

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/refactoring-runtime-package-with-builtin-5409af54
Apr 27, 2026
Merged

refactor(runtime): consolidate hook orchestration and cache executors#2523
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/refactoring-runtime-package-with-builtin-5409af54

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Apr 27, 2026

Summary

Continues the hooks-extraction refactoring: pulls more orchestration out of pkg/runtime, makes the pkg/hooks mechanism more powerful, and significantly simplifies the runtime package.

Net effect across pkg/runtime: −189 lines, all hook code colocated in a single hooks.go, no behaviour change. go test -race and golangci-lint both clean.

Changes

pkg/hooks — more ergonomic for builtin authors and embedders

  • Executor.Dispatch now auto-fills Input.Cwd from the executor's working directory when callers leave it empty. Eliminates the per-callsite Cwd: r.workingDir plumbing across the runtime.
  • New hooks.NewAdditionalContextOutput(event, content) helper for BuiltinFunc implementations that just want to contribute additional context. Returns nil for empty content.

pkg/hooks/builtins — owns the agent-flag → builtin wiring

  • New AgentDefaults struct + ApplyAgentDefaults(cfg, defaults) *hooks.Config. Encodes the rules:
    • AddDateturn_start (recompute every turn)
    • AddPromptFilesturn_start (file may be edited mid-session)
    • AddEnvironmentInfosession_start (cwd / OS / arch are session-stable)
  • Returns nil when the resulting config is empty so the runtime can skip executor construction. Covered by three new tests.
  • Builtins now use NewAdditionalContextOutput, dropping duplicated Output literal construction.

pkg/runtime — simpler and faster

  • New pkg/runtime/hooks.go holds all hook orchestration:
    • hooksExec(a) replaces getHooksExecutor and caches the per-agent *hooks.Executor (including the nil "no hooks configured" sentinel) under an RWMutex. Per-turn dispatches no longer pay the matcher-compilation cost repeatedly.
    • dispatchHook(ctx, agent, event, input, events) is the single shared dispatcher: resolves the cached executor, short-circuits on no-hook, dispatches, logs errors, and emits Warning events for SystemMessage.
  • The nine executeXHooks methods shrink from 10–25 lines each to a handful of lines, all delegating to dispatchHook.
  • tool_dispatch.go: runTool no longer fetches the executor; pre/post-tool hooks now go through dispatchHook too. newHooksInput is a package function and no longer sets Cwd.

Diffstat

```
pkg/config/latest/hooks_yaml_test.go | 4 +-
pkg/hooks/builtins/builtins.go | 65 +++++--
pkg/hooks/builtins/builtins_test.go | 54 ++++++
pkg/hooks/executor.go | 6 +-
pkg/hooks/types.go | 16 ++
pkg/runtime/hooks.go | 230 +++++++++++++++++++++++++
pkg/runtime/hooks_wiring_test.go | 20 ++-
pkg/runtime/runtime.go | 317 +----------------------------------
pkg/runtime/tool_dispatch.go | 71 ++++----
9 files changed, 412 insertions(+), 371 deletions(-)
```

Validation

  • `go build ./...` ✅
  • `go vet ./...` ✅
  • `go test ./...` ✅
  • `go test -race ./pkg/runtime/... ./pkg/hooks/...` ✅
  • `golangci-lint run ./...` → 0 issues

Assisted-By: docker-agent

Continue the hooks-extraction refactoring by pulling more orchestration
out of pkg/runtime, making the hooks mechanism more powerful, and
simplifying the runtime package.

- Add hooks.NewAdditionalContextOutput helper and auto-fill Input.Cwd
  from the executor's working directory in Dispatch, removing repeated
  boilerplate at every callsite.
- Move agent-flag to builtin auto-injection into pkg/hooks/builtins as
  AgentDefaults / ApplyAgentDefaults, with dedicated tests.
- Introduce pkg/runtime/hooks.go with a cached per-agent hooksExec and
  a single dispatchHook helper. The nine executeXHooks methods now
  delegate to it, shrinking each to a few lines.
- Rewire tool_dispatch.go's pre/post-tool paths through dispatchHook so
  they share the same dispatch + SystemMessage handling.
- Update hooks_wiring_test.go to cover the executor cache and rename
  the test to TestHooksExecWiresAgentFlagsToBuiltins.

Net effect: -189 lines across runtime + tool_dispatch, all hook code
centralised in hooks.go, no behavior change. Tests pass with -race;
golangci-lint reports zero issues.

Assisted-By: docker-agent
@dgageot dgageot requested a review from a team as a code owner April 27, 2026 11:12
@dgageot dgageot merged commit 3fcc3e7 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