feat(hooks/script): integrate script handler — FireResult + source-tier gate + migration

Wave 1 Phase 03. Wires the Phase 02 Goja handler into the dispatcher, widens
the DB schema for script + builtin sources, and refactors Dispatcher.Fire to
return a FireResult so builtin-source hooks can mutate event input.

Dispatcher:
- FireResult{Decision, UpdatedToolInput, UpdatedRawInput} replaces the plain
  Decision return. stdDispatcher.runSync keeps a local evMut copy so a
  builtin-source hook's updatedInput flows to downstream hooks in the chain
  and to the caller; non-builtin (source=ui) script mutations are dropped +
  WARN-logged (defense-in-depth; source tier enforced at the dispatcher, not
  only at the handler).
- applyBuiltinMutation + placeholder builtinAllowlistFor (rawInput, toolInput)
  — Phase 04 overrides the allowlist from builtins.yaml.
- noopDispatcher returns FireResult{DecisionAllow}.
- Script mutation tests: TestDispatcher_ScriptMutation_BuiltinSourceApplies,
  TestDispatcher_ScriptMutation_UISourceDenied.

Pipeline + callers (14 Fire sites refactored):
- FireHook wrapper returns FireResult; context_stage.go:52 applies
  UpdatedRawInput → state.Input.Message; tool_stage.go:52 applies
  UpdatedToolInput → tc.Arguments before ExecuteToolCall.
- delegate_bridge + delegate_tool keep the Decision branch; Updated* ignored.
- dispatcher_test / delegate_bridge_test / delegate_tool_hooks_test /
  integration test helpers updated for the new shape.

Validation:
- edition_gate allows HandlerScript on every edition (sandboxed, no shell
  escape surface).
- config.validateHandler rejects empty source, source > 32 KiB, goja compile
  error; validateTimeout rejects on_timeout=ask|defer (reserved).
- Six new config tests cover the script path.

Migrations:
- PG migration 000053 relaxes handler_type + source CHECKs and drops
  uq_hooks_{global,tenant,agent} — scripts routinely want many small hooks
  per event. RequiredSchemaVersion → 53.
- SQLite SchemaVersion → 21; patch 20 is a SELECT 1 placeholder because
  SQLite can't ALTER a CHECK — rebuildAgentHooksV21 runs outside the
  migration tx (parallel to backfillV16) to rename/recreate the table with
  widened CHECKs. schema.sql fresh-DB path matches.
- H9 fix: PGHookStore.Create + SqliteHookStore.Create honor caller-provided
  cfg.ID (uuid.Nil falls back to UUIDv7), unblocking Phase 04 idempotent
  UUIDv5 seed. TestCreateHonorsFixedID on both stores.

Config:
- config.HooksConfig{ScriptConcurrency, ScriptPerTenantConcurrency,
  ScriptCacheSize, BuiltinDisable}; buildHookHandlers wires the script
  handler with the caps from appCfg.Hooks.

Gates green: go build ./... + sqliteonly, go test -race -count=1 across
hooks/pipeline/sqlitestore.
This commit is contained in:
viettranx committed 2026-04-16 14:17:47 +07:00
1 parent a556161d0d
commit b7592bcacf
29 files changed
+663 -125

No files matched your search

+12 -1
View File
@@ -4,6 +4,7 @@ import (
"os"
"time"
"github.com/nextlevelbuilder/goclaw/internal/config"
"github.com/nextlevelbuilder/goclaw/internal/edition"
"github.com/nextlevelbuilder/goclaw/internal/hooks"
hookhandlers "github.com/nextlevelbuilder/goclaw/internal/hooks/handlers"
@@ -26,7 +27,7 @@ var sharedHookHandlers map[hooks.HandlerType]hooks.Handler
// Budget wiring (C1 fix): the PromptHandler receives a budget.Store bound
// to pg.NewPGHookBudget so token spend is atomically deducted per tenant.
// When the DB handle is unavailable, budget falls back to nil (Lite desktop).
func buildHookHandlers(stores *store.Stores, providerReg *providers.Registry) map[hooks.HandlerType]hooks.Handler {
func buildHookHandlers(stores *store.Stores, providerReg *providers.Registry, hooksCfg config.HooksConfig) map[hooks.HandlerType]hooks.Handler {
encryptKey := os.Getenv("GOCLAW_ENCRYPTION_KEY")
var budgetStore *budget.Store
@@ -40,6 +41,15 @@ func buildHookHandlers(stores *store.Stores, providerReg *providers.Registry) ma
DefaultModel: "haiku",
}
// ScriptHandler: bounded by cfg.Hooks caps; zero values fall back to
// handler defaults (10 / 3 / 500). Safe for concurrent reuse across
// dispatcher + hooks.test runner (each Execute allocates its own runtime).
scriptHandler := hookhandlers.NewScriptHandler(
hooksCfg.ScriptConcurrency,
hooksCfg.ScriptPerTenantConcurrency,
hooksCfg.ScriptCacheSize,
)
return map[hooks.HandlerType]hooks.Handler{
hooks.HandlerCommand: &hookhandlers.CommandHandler{Edition: edition.Current()},
hooks.HandlerHTTP: &hookhandlers.HTTPHandler{
@@ -47,5 +57,6 @@ func buildHookHandlers(stores *store.Stores, providerReg *providers.Registry) ma
Client: security.NewSafeClient(10 * time.Second),
},
hooks.HandlerPrompt: promptHandler,
hooks.HandlerScript: scriptHandler,
}
}
+1 -1
View File
@@ -148,7 +148,7 @@ func wireExtras(
// Agent Hooks (Issue #875) — lifecycle dispatcher + handlers.
var hookDispatcher hooks.Dispatcher = hooks.NewNoopDispatcher()
if hs, ok := stores.Hooks.(hooks.HookStore); ok && hs != nil {
handlers := buildHookHandlers(stores, providerReg)
handlers := buildHookHandlers(stores, providerReg, appCfg.Hooks)
stdOpts := hooks.StdDispatcherOpts{
Store: hs,
Audit: hooks.NewAuditWriter(hs, ""),
+18
View File
@@ -55,9 +55,27 @@ type Config struct {
Telemetry TelemetryConfig `json:"telemetry"`
Tailscale TailscaleConfig `json:"tailscale"`
Bindings []AgentBinding `json:"bindings,omitempty"`
Hooks HooksConfig `json:"hooks,omitempty"`
mu sync.RWMutex
}
// HooksConfig tunes the script-hook runtime caps. All zero-valued fields fall
// back to the handlers package defaults (see handlers.NewScriptHandler).
//
// ScriptConcurrency bounds the total concurrent script executions per process.
// ScriptPerTenantConcurrency bounds a single tenant's share of that pool so a
// runaway tenant cannot starve global slots. ScriptCacheSize caps the LRU of
// compiled goja programs keyed by (hookID, version).
//
// BuiltinDisable names builtin hook IDs (from Phase 04's builtins.yaml) that
// the operator wants force-disabled at startup — per-deployment escape hatch.
type HooksConfig struct {
ScriptConcurrency int `json:"script_concurrency,omitempty"`
ScriptPerTenantConcurrency int `json:"script_per_tenant_concurrency,omitempty"`
ScriptCacheSize int `json:"script_cache_size,omitempty"`
BuiltinDisable []string `json:"builtin_disable,omitempty"`
}
// TailscaleConfig configures the optional Tailscale tsnet listener.
// Requires building with -tags tsnet. Auth key from env only (never persisted).
type TailscaleConfig struct {
+29 -1
View File
@@ -3,6 +3,7 @@ package hooks
import (
"fmt"
"github.com/dop251/goja"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/edition"
@@ -121,6 +122,19 @@ func (h *HookConfig) validateHandler(ed edition.Edition) error {
if tmpl, _ := h.Config["prompt_template"].(string); tmpl == "" {
return fmt.Errorf("hook: prompt handler requires non-empty prompt_template")
}
case HandlerScript:
source, _ := h.Config["source"].(string)
if source == "" {
return fmt.Errorf("hook: script handler requires non-empty config.source")
}
if len(source) > MaxScriptSourceBytes {
return fmt.Errorf("hook: script source exceeds %d bytes (got %d)", MaxScriptSourceBytes, len(source))
}
// Strict=true parallels the handler's compile; surfaces `Line N:M`
// errors that the UI editor can display inline.
if _, err := goja.Compile(h.ID.String(), source, true); err != nil {
return fmt.Errorf("hook: script compile error: %w", err)
}
case HandlerCommand, HandlerHTTP:
// No extra filter required — matcher/if_expr are optional.
default:
@@ -129,7 +143,14 @@ func (h *HookConfig) validateHandler(ed edition.Edition) error {
return nil
}
// validateTimeout bounds the per-hook timeout within [0, MaxTimeoutMS].
// MaxScriptSourceBytes mirrors handlers.MaxScriptSourceBytes (32 KiB). Declared
// here to avoid importing the handlers package (which imports goja) just for
// this constant — the size cap is part of the config contract and should be
// enforceable without the runtime dep elsewhere.
const MaxScriptSourceBytes = 32 * 1024
// validateTimeout bounds the per-hook timeout within [0, MaxTimeoutMS] and
// rejects on_timeout values reserved for future phases.
// Zero is normalized to DefaultTimeoutMS in applyDefaults.
func (h *HookConfig) validateTimeout() error {
if h.TimeoutMS < 0 {
@@ -138,6 +159,13 @@ func (h *HookConfig) validateTimeout() error {
if h.TimeoutMS > MaxTimeoutMS {
return fmt.Errorf("hook: timeout_ms %d exceeds max %d", h.TimeoutMS, MaxTimeoutMS)
}
// ask/defer are reserved for Wave 3 human-approval + external-arbitration
// flows. Until the dispatcher wires a `request_user_input` path, accepting
// these as timeout-fallback would let a misconfigured hook hang blocking
// events indefinitely.
if h.OnTimeout == DecisionAsk || h.OnTimeout == DecisionDefer {
return fmt.Errorf("hook: on_timeout=%q not supported in Wave 1 (reserved for future)", h.OnTimeout)
}
return nil
}
+63
View File
@@ -254,3 +254,66 @@ func TestValidate_CapsTimeoutToMax(t *testing.T) {
t.Fatal("expected error for timeout > MaxTimeoutMS")
}
}
// ─── Script handler validation (Phase 03) ────────────────────────────────────
func baseValidScriptHook() hooks.HookConfig {
h := baseValidCommandHook()
h.HandlerType = hooks.HandlerScript
h.Config = map[string]any{"source": `function handle(e){return {decision:"allow"}}`}
return h
}
func TestValidate_AcceptsValidScriptHook(t *testing.T) {
h := baseValidScriptHook()
if err := h.Validate(edition.Standard); err != nil {
t.Fatalf("expected nil error; got %v", err)
}
}
func TestValidate_RejectsEmptyScriptSource(t *testing.T) {
h := baseValidScriptHook()
h.Config = map[string]any{"source": ""}
err := h.Validate(edition.Standard)
if err == nil || !strings.Contains(err.Error(), "non-empty") {
t.Fatalf("expected non-empty source error; got %v", err)
}
}
func TestValidate_RejectsOversizedScriptSource(t *testing.T) {
h := baseValidScriptHook()
big := strings.Repeat("// pad\n", 6000) // >32 KiB
h.Config = map[string]any{"source": big}
err := h.Validate(edition.Standard)
if err == nil || !strings.Contains(err.Error(), "exceeds") {
t.Fatalf("expected exceeds-bytes error; got %v", err)
}
}
func TestValidate_RejectsScriptCompileError(t *testing.T) {
h := baseValidScriptHook()
// Missing closing brace → parser error with line:col.
h.Config = map[string]any{"source": `function handle(e) { return {`}
err := h.Validate(edition.Standard)
if err == nil || !strings.Contains(err.Error(), "compile error") {
t.Fatalf("expected compile error; got %v", err)
}
}
func TestValidate_RejectsOnTimeoutAsk(t *testing.T) {
h := baseValidScriptHook()
h.OnTimeout = hooks.DecisionAsk
err := h.Validate(edition.Standard)
if err == nil || !strings.Contains(err.Error(), "on_timeout") {
t.Fatalf("expected on_timeout rejection; got %v", err)
}
}
func TestValidate_RejectsOnTimeoutDefer(t *testing.T) {
h := baseValidScriptHook()
h.OnTimeout = hooks.DecisionDefer
err := h.Validate(edition.Standard)
if err == nil || !strings.Contains(err.Error(), "on_timeout") {
t.Fatalf("expected on_timeout rejection; got %v", err)
}
}
+2
View File
@@ -45,6 +45,8 @@ func SubscribeDelegateEvents(bus eventbus.DomainEventBus, d Dispatcher) {
"err", err,
)
}
// SubagentStop is non-blocking; Updated* from FireResult is ignored
// (delegate bridge has no mutation path in Wave 1).
return nil
}
+2 -2
View File
@@ -40,9 +40,9 @@ type fakeCapturingDispatcher struct {
events []Event
}
func (f *fakeCapturingDispatcher) Fire(_ context.Context, ev Event) (Decision, error) {
func (f *fakeCapturingDispatcher) Fire(_ context.Context, ev Event) (FireResult, error) {
f.events = append(f.events, ev)
return DecisionAllow, nil
return FireResult{Decision: DecisionAllow}, nil
}
// --- tests ---
+129 -21
View File
@@ -20,9 +20,11 @@ type Handler interface {
}
// Dispatcher is the pipeline integration surface. Stages call Fire and act on
// the Decision (allow → continue; block → abort with i18n error).
// FireResult.Decision (allow → continue; block → abort with i18n error).
// FireResult.Updated* carry mutations authored by builtin-source hooks —
// callers apply them to their own tc.Arguments / state.Input when non-nil.
type Dispatcher interface {
Fire(ctx context.Context, ev Event) (Decision, error)
Fire(ctx context.Context, ev Event) (FireResult, error)
}
// MaxLoopDepth caps nested hook invocation (M5). Depth increments when a hook
@@ -130,8 +132,8 @@ func NewNoopDispatcher() Dispatcher {
type noopDispatcher struct{}
func (noopDispatcher) Fire(context.Context, Event) (Decision, error) {
return DecisionAllow, nil
func (noopDispatcher) Fire(context.Context, Event) (FireResult, error) {
return FireResult{Decision: DecisionAllow}, nil
}
// ── stdDispatcher ────────────────────────────────────────────────────────────
@@ -146,13 +148,13 @@ type stdDispatcher struct {
cb *circuitBreaker
}
func (d *stdDispatcher) Fire(ctx context.Context, ev Event) (Decision, error) {
func (d *stdDispatcher) Fire(ctx context.Context, ev Event) (FireResult, error) {
if depthFromCtx(ctx) > MaxLoopDepth {
slog.Warn("security.hook.loop_depth_exceeded",
"event_id", ev.EventID,
"hook_event", ev.HookEvent,
)
return DecisionError, ErrLoopDepthExceeded
return FireResult{Decision: DecisionError}, ErrLoopDepthExceeded
}
hooks, err := d.store.ResolveForEvent(ctx, ev)
@@ -160,68 +162,174 @@ func (d *stdDispatcher) Fire(ctx context.Context, ev Event) (Decision, error) {
// Fail-closed: a DB blip must not let a pre-tool gate open silently.
slog.Warn("security.hook.resolve_error", "err", err, "event_id", ev.EventID)
if ev.HookEvent.IsBlocking() {
return DecisionBlock, err
return FireResult{Decision: DecisionBlock}, err
}
return DecisionAllow, err
return FireResult{Decision: DecisionAllow}, err
}
if len(hooks) == 0 {
return DecisionAllow, nil
return FireResult{Decision: DecisionAllow}, nil
}
if ev.HookEvent.IsBlocking() {
return d.runSync(ctx, ev, hooks)
}
d.runAsync(ctx, ev, hooks)
return DecisionAllow, nil
return FireResult{Decision: DecisionAllow}, nil
}
// runSync executes the blocking chain with a wall-time budget and per-hook
// timeouts. First block wins; any fail-closed condition aborts to Block.
func (d *stdDispatcher) runSync(ctx context.Context, ev Event, chain []HookConfig) (Decision, error) {
//
// Mutation propagation (Phase 03): the dispatcher keeps a local evMut copy
// with its own ToolInput map so a builtin-source hook's updatedInput becomes
// visible to downstream hooks in the chain (and to the caller via FireResult)
// without leaking mutations back into the caller's original event on error
// paths. Non-builtin scripts returning updatedInput get their mutation
// dropped + logged — source tier enforced at the dispatcher.
func (d *stdDispatcher) runSync(ctx context.Context, ev Event, chain []HookConfig) (FireResult, error) {
chainCtx, cancel := context.WithTimeout(ctx, d.chainBudget)
defer cancel()
evMut := ev
if ev.ToolInput != nil {
evMut.ToolInput = cloneMap(ev.ToolInput)
}
mutated := false
for _, cfg := range chain {
if !cfg.Enabled {
continue
}
if d.cb.isTripped(cfg.ID, d.now()) {
d.writeExec(ctx, cfg, ev, DecisionBlock, 0, "circuit breaker open")
return DecisionBlock, nil
d.writeExec(ctx, cfg, evMut, DecisionBlock, 0, "circuit breaker open")
return FireResult{Decision: DecisionBlock}, nil
}
if !d.prefilter(cfg, ev) {
if !d.prefilter(cfg, evMut) {
continue
}
dec, execErr, duration := d.runOne(chainCtx, cfg, ev)
// Attach fresh ScriptResult carrier per-hook so the script handler can
// report its non-decision outputs (reason/updatedInput/stdout). Other
// handler types ignore the ctx value — zero-cost.
scriptRes := &ScriptResult{}
hctx := WithScriptResult(chainCtx, scriptRes)
dec, execErr, duration := d.runOne(hctx, cfg, evMut)
errMsg := ""
if execErr != nil {
errMsg = execErr.Error()
}
d.writeExec(ctx, cfg, ev, dec, duration, errMsg)
d.writeExec(ctx, cfg, evMut, dec, duration, errMsg)
// Apply mutation to the local copy only when the hook is builtin-source.
// Non-builtin scripts get their updatedInput stripped + warned.
if cfg.HandlerType == HandlerScript && dec == DecisionAllow && scriptRes.UpdatedInput != nil {
if cfg.Source == SourceBuiltin {
applyBuiltinMutation(&evMut, scriptRes.UpdatedInput, builtinAllowlistFor(cfg.ID))
mutated = true
} else {
slog.Warn("hooks.script_mutation_denied",
"hook_id", cfg.ID,
"source", cfg.Source,
"field_count", len(scriptRes.UpdatedInput),
)
}
}
switch dec {
case DecisionBlock:
d.cb.record(cfg.ID, d.now(), d.store)
return DecisionBlock, nil
return FireResult{Decision: DecisionBlock}, nil
case DecisionTimeout:
d.cb.record(cfg.ID, d.now(), d.store)
if cfg.OnTimeout == DecisionBlock {
return DecisionBlock, nil
return FireResult{Decision: DecisionBlock}, nil
}
// OnTimeout=allow: degrade gracefully but keep scanning.
case DecisionError:
// Unexpected error in a blocking chain → fail-closed.
return DecisionBlock, nil
return FireResult{Decision: DecisionBlock}, nil
}
if chainCtx.Err() != nil {
// Chain wall-time budget exhausted (H3): fail-closed.
return DecisionBlock, nil
return FireResult{Decision: DecisionBlock}, nil
}
}
return DecisionAllow, nil
result := FireResult{Decision: DecisionAllow}
if mutated {
if evMut.ToolInput != nil {
result.UpdatedToolInput = evMut.ToolInput
}
if evMut.RawInput != ev.RawInput {
s := evMut.RawInput
result.UpdatedRawInput = &s
}
}
return result, nil
}
// cloneMap returns a shallow copy of m. Used so runSync's mutations on the
// local evMut don't leak back to the caller's event when a downstream hook
// blocks or the chain aborts.
func cloneMap(m map[string]any) map[string]any {
out := make(map[string]any, len(m))
for k, v := range m {
out[k] = v
}
return out
}
// applyBuiltinMutation applies the builtin hook's updatedInput fields into ev,
// gated by the per-builtin allowlist. The allowlist is loaded in Phase 04 from
// builtins.yaml; Phase 03 uses a permissive default (rawInput + toolInput).
func applyBuiltinMutation(ev *Event, updated map[string]any, allowlist []string) {
if updated == nil {
return
}
allowSet := make(map[string]struct{}, len(allowlist))
for _, f := range allowlist {
allowSet[f] = struct{}{}
}
if _, ok := allowSet["rawInput"]; ok {
if s, ok2 := updated["rawInput"].(string); ok2 {
ev.RawInput = s
}
}
if m, ok := updated["toolInput"].(map[string]any); ok {
if _, wildcard := allowSet["toolInput"]; wildcard {
if ev.ToolInput == nil {
ev.ToolInput = map[string]any{}
}
for k, v := range m {
ev.ToolInput[k] = v
}
} else {
for k, v := range m {
if _, ok := allowSet["toolInput."+k]; ok {
if ev.ToolInput == nil {
ev.ToolInput = map[string]any{}
}
ev.ToolInput[k] = v
}
}
}
}
}
// builtinAllowlistFor returns the field allowlist for a builtin hook.
// Phase 03 ships a permissive default; Phase 04 overrides with a per-hook
// allowlist loaded from builtins.yaml at startup.
func builtinAllowlistFor(_ uuid.UUID) []string {
return []string{"rawInput", "toolInput"}
}
// SourceBuiltin is the `source` value marking a hook seeded by the builtin
// infrastructure (Phase 04 loader). Only builtin-source hooks are allowed to
// mutate event input through the script handler's updatedInput field.
const SourceBuiltin = "builtin"
// runAsync fires non-blocking hooks concurrently. Phase 1 uses a simple
// goroutine-per-hook; Phase 2 will route through the eventbus worker pool.
func (d *stdDispatcher) runAsync(ctx context.Context, ev Event, chain []HookConfig) {
+103 -24
View File
@@ -122,15 +122,15 @@ func TestDispatcher_NoHooks_ReturnsAllow(t *testing.T) {
Store: fs,
Audit: hooks.NewAuditWriter(fs, ""),
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e1",
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", r.Decision)
}
}
@@ -157,15 +157,15 @@ func TestDispatcher_SyncChain_FirstBlockWins(t *testing.T) {
},
},
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-block",
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block", r.Decision)
}
if atomic.LoadInt32(&lateHandler.calls) != 0 {
t.Errorf("post-block handler ran %d times, want 0", lateHandler.calls)
@@ -198,15 +198,15 @@ func TestDispatcher_PerHookTimeout_FailsClosed(t *testing.T) {
PerHookTimeout: 20 * time.Millisecond,
ChainBudget: 5 * time.Second,
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-timeout",
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on timeout)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on timeout)", r.Decision)
}
// Exactly one execution row recorded with timeout decision.
execs := fs.snapshotExecs()
@@ -281,15 +281,15 @@ func TestDispatcher_NonBlockingEvent_AsyncNoDecision(t *testing.T) {
}),
},
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-async",
HookEvent: hooks.EventPostToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow (non-blocking path never blocks)", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow (non-blocking path never blocks)", r.Decision)
}
select {
case <-done:
@@ -314,15 +314,15 @@ func TestDispatcher_AllAllow_ReturnsAllow(t *testing.T) {
Audit: hooks.NewAuditWriter(fs, ""),
Handlers: map[hooks.HandlerType]hooks.Handler{hooks.HandlerHTTP: &fakeHandler{decision: hooks.DecisionAllow}},
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-all-allow",
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", r.Decision)
}
if got := len(fs.snapshotExecs()); got != 2 {
t.Errorf("exec rows=%d, want 2 (one per hook)", got)
@@ -335,15 +335,15 @@ func TestDispatcher_ResolveError_FailsClosed(t *testing.T) {
Store: fs,
Audit: hooks.NewAuditWriter(fs, ""),
})
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-resolve-err",
HookEvent: hooks.EventPreToolUse,
})
if err == nil {
t.Fatal("Fire err=nil, want non-nil on resolve failure")
}
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed)", r.Decision)
}
}
@@ -356,12 +356,12 @@ func TestDispatcher_MissingHandler_BlocksBlockingEvent(t *testing.T) {
Store: fs,
Audit: hooks.NewAuditWriter(fs, ""),
})
dec, _ := d.Fire(context.Background(), hooks.Event{
r, _ := d.Fire(context.Background(), hooks.Event{
EventID: "e-no-handler",
HookEvent: hooks.EventPreToolUse,
})
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (missing handler → fail-closed)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (missing handler → fail-closed)", r.Decision)
}
}
@@ -388,14 +388,93 @@ func TestDispatcher_DedupKey_IncludesEventID(t *testing.T) {
func TestNoopDispatcher_AlwaysAllows(t *testing.T) {
d := hooks.NewNoopDispatcher()
dec, err := d.Fire(context.Background(), hooks.Event{
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e",
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", r.Decision)
}
if r.UpdatedToolInput != nil || r.UpdatedRawInput != nil {
t.Errorf("noop must return zero Updated* (got ti=%v ri=%v)", r.UpdatedToolInput, r.UpdatedRawInput)
}
}
// mutatingHandler writes an UpdatedInput to the ctx-carried ScriptResult so
// the dispatcher's source-tier gate can be exercised without invoking the
// real goja runtime.
type mutatingHandler struct {
updated map[string]any
}
func (h *mutatingHandler) Execute(ctx context.Context, _ hooks.HookConfig, _ hooks.Event) (hooks.Decision, error) {
if r := hooks.ScriptResultFrom(ctx); r != nil {
r.UpdatedInput = h.updated
}
return hooks.DecisionAllow, nil
}
// TestDispatcher_ScriptMutation_BuiltinSourceApplies verifies the source-tier
// capability: a script hook with source="builtin" has its UpdatedInput applied
// to FireResult.Updated*; a script hook with source="ui" does NOT.
func TestDispatcher_ScriptMutation_BuiltinSourceApplies(t *testing.T) {
cfg := newBaseHook(hooks.HandlerScript, hooks.EventUserPromptSubmit)
cfg.Source = hooks.SourceBuiltin
fs := &fakeStore{hooks: []hooks.HookConfig{cfg}}
h := &mutatingHandler{updated: map[string]any{"rawInput": "REDACTED"}}
d := hooks.NewStdDispatcher(hooks.StdDispatcherOpts{
Store: fs,
Audit: hooks.NewAuditWriter(fs, ""),
Handlers: map[hooks.HandlerType]hooks.Handler{hooks.HandlerScript: h},
})
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-builtin",
HookEvent: hooks.EventUserPromptSubmit,
RawInput: "ssn is 123-45-6789",
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if r.Decision != hooks.DecisionAllow {
t.Fatalf("decision=%q, want allow", r.Decision)
}
if r.UpdatedRawInput == nil {
t.Fatal("UpdatedRawInput nil; expected redacted rawInput from builtin hook")
}
if *r.UpdatedRawInput != "REDACTED" {
t.Errorf("UpdatedRawInput=%q, want REDACTED", *r.UpdatedRawInput)
}
}
func TestDispatcher_ScriptMutation_UISourceDenied(t *testing.T) {
cfg := newBaseHook(hooks.HandlerScript, hooks.EventUserPromptSubmit)
cfg.Source = "ui"
fs := &fakeStore{hooks: []hooks.HookConfig{cfg}}
h := &mutatingHandler{updated: map[string]any{"rawInput": "pwn"}}
d := hooks.NewStdDispatcher(hooks.StdDispatcherOpts{
Store: fs,
Audit: hooks.NewAuditWriter(fs, ""),
Handlers: map[hooks.HandlerType]hooks.Handler{hooks.HandlerScript: h},
})
r, err := d.Fire(context.Background(), hooks.Event{
EventID: "e-ui",
HookEvent: hooks.EventUserPromptSubmit,
RawInput: "original",
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if r.Decision != hooks.DecisionAllow {
t.Fatalf("decision=%q, want allow", r.Decision)
}
if r.UpdatedRawInput != nil {
t.Fatalf("non-builtin mutation leaked: UpdatedRawInput=%v", *r.UpdatedRawInput)
}
}
+3 -1
View File
@@ -15,7 +15,9 @@ type HookEditionPolicy struct{}
// reason is a stable string suitable for audit rows and i18n key lookup.
func (HookEditionPolicy) Allow(ht HandlerType, _ Scope, ed edition.Edition) (bool, string) {
switch ht {
case HandlerHTTP, HandlerPrompt:
case HandlerHTTP, HandlerPrompt, HandlerScript:
// Script hooks run in a sandboxed goja runtime — no shell escape surface.
// Allowed on every edition for tenant admin + operator authors alike.
return true, ""
case HandlerCommand:
if ed.Name == edition.Lite.Name {
+19
View File
@@ -134,6 +134,25 @@ func (d Decision) IsBlock() bool {
return d == DecisionBlock
}
// FireResult is the return value of Dispatcher.Fire. Callers read Decision to
// branch on allow/block and apply Updated* when a builtin hook mutated input.
//
// UpdatedToolInput is non-nil only when at least one builtin-source hook in
// the chain returned updatedInput AND the dispatcher applied allow-listed
// fields. Callers overwrite their own tc.Arguments / state.Input with it.
//
// UpdatedRawInput points to a string only when a builtin hook mutated
// rawInput. Callers replace state.Input.Message with the dereferenced value.
//
// For non-builtin scripts returning updatedInput the dispatcher strips the
// mutation + logs a WARN; Updated* stay nil (defense-in-depth against a
// tenant-authored script escalating its capability tier).
type FireResult struct {
Decision Decision
UpdatedToolInput map[string]any
UpdatedRawInput *string
}
// ─── Config & execution structs ──────────────────────────────────────────────
// HookConfig mirrors the agent_hooks DB row. All pointer fields correspond to
+7 -3
View File
@@ -48,16 +48,20 @@ func (s *ContextStage) Execute(ctx context.Context, state *RunState) error {
})
}
// Hook: sync UserPromptSubmit — blocking; abort if blocked.
if dec, _ := s.deps.FireHook(ctx, hooks.Event{
// Hook: sync UserPromptSubmit — blocking; abort if blocked. A builtin-
// source hook may rewrite RawInput (e.g. PII redactor); apply the update
// before any downstream stage reads state.Input.Message.
if r, _ := s.deps.FireHook(ctx, hooks.Event{
EventID: uuid.NewString(),
SessionID: state.Input.SessionKey,
TenantID: store.TenantIDFromContext(ctx),
AgentID: store.AgentIDFromContext(ctx),
RawInput: state.Input.Message,
HookEvent: hooks.EventUserPromptSubmit,
}); dec == hooks.DecisionBlock {
}); r.Decision == hooks.DecisionBlock {
return fmt.Errorf("hook blocked user_prompt_submit")
} else if r.UpdatedRawInput != nil {
state.Input.Message = *r.UpdatedRawInput
}
// 0. Inject context values (agent/tenant/user/workspace scoping, input guard, truncation).
+6 -4
View File
@@ -110,11 +110,13 @@ type PipelineDeps struct {
MaybeSummarize func(ctx context.Context, sessionKey string)
}
// FireHook is nil-safe. Returns DecisionAllow when no dispatcher is wired.
// Callers on blocking events should abort the stage on DecisionBlock.
func (d *PipelineDeps) FireHook(ctx context.Context, ev hooks.Event) (hooks.Decision, error) {
// FireHook is nil-safe. Returns FireResult{Decision: DecisionAllow} when no
// dispatcher is wired. Callers on blocking events should abort the stage on
// result.Decision == DecisionBlock and apply result.UpdatedToolInput /
// UpdatedRawInput to their local state when non-nil (builtin-hook mutations).
func (d *PipelineDeps) FireHook(ctx context.Context, ev hooks.Event) (hooks.FireResult, error) {
if d == nil || d.Hooks == nil {
return hooks.DecisionAllow, nil
return hooks.FireResult{Decision: hooks.DecisionAllow}, nil
}
return d.Hooks.Fire(ctx, ev)
}
+7 -3
View File
@@ -48,8 +48,10 @@ func (s *ToolStage) Execute(ctx context.Context, state *RunState) error {
// Sequential fallback: ExecuteToolCall handles both I/O and state mutation.
for _, tc := range toolCalls {
// Hook: sync PreToolUse — block if hook denies.
if dec, _ := s.deps.FireHook(ctx, hooks.Event{
// Hook: sync PreToolUse — block if hook denies. Builtin-source hooks may
// rewrite tc.Arguments via UpdatedToolInput (e.g. path-sanitizer); apply
// before ExecuteToolCall so the rewrite is authoritative.
if r, _ := s.deps.FireHook(ctx, hooks.Event{
EventID: uuid.NewString(),
SessionID: state.Input.SessionKey,
TenantID: store.TenantIDFromContext(ctx),
@@ -57,7 +59,7 @@ func (s *ToolStage) Execute(ctx context.Context, state *RunState) error {
ToolName: tc.Name,
ToolInput: tc.Arguments,
HookEvent: hooks.EventPreToolUse,
}); dec == hooks.DecisionBlock {
}); r.Decision == hooks.DecisionBlock {
// Inject synthetic blocked tool message and skip actual execution.
state.Messages.AppendPending(providers.Message{
Role: "tool",
@@ -66,6 +68,8 @@ func (s *ToolStage) Execute(ctx context.Context, state *RunState) error {
})
state.Tool.TotalToolCalls++
continue
} else if r.UpdatedToolInput != nil {
tc.Arguments = r.UpdatedToolInput
}
msgs, err := s.deps.ExecuteToolCall(ctx, state, tc)
+8 -1
View File
@@ -43,7 +43,14 @@ func NewPGHookStore(db *sql.DB) *PGHookStore {
// ─── Create ─────────────────────────────────────────────────────────────────
func (s *PGHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uuid.UUID, error) {
id := uuid.Must(uuid.NewV7())
// Honor a caller-provided fixed ID when non-nil (H9 fix) — the Phase 04
// builtin seeder needs idempotent UPSERTs keyed by UUIDv5, and tests need
// deterministic IDs. Fall back to UUIDv7 only when the caller did not
// supply one.
id := cfg.ID
if id == uuid.Nil {
id = uuid.Must(uuid.NewV7())
}
now := time.Now().UTC()
cfgJSON, err := json.Marshal(cfg.Config)
+35
View File
@@ -381,3 +381,38 @@ func TestPGHookStore_CacheInvalidatedOnWrite(t *testing.T) {
t.Errorf("expected more hooks after create: before=%d after=%d", beforeCount, len(after))
}
}
// H9 (Phase 03): Create must honor a caller-supplied cfg.ID so the builtin
// seeder's idempotent UUIDv5 keys survive restarts and tests can use
// deterministic IDs. Verified on both Create paths (fixed + auto).
func TestPGHookStore_CreateHonorsFixedID(t *testing.T) {
db := hooksTestDB(t)
tenantID, _ := seedTenantAndAgent(t, db)
s := NewPGHookStore(db)
ctx := tenantScopedCtx(tenantID)
fixed := uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
cfg := minimalHook(tenantID, hooks.EventUserPromptSubmit)
cfg.ID = fixed
got, err := s.Create(ctx, cfg)
if err != nil {
t.Fatalf("Create fixed id: %v", err)
}
t.Cleanup(func() { s.Delete(masterCtx(), got) })
if got != fixed {
t.Fatalf("returned id=%s, want %s (H9: caller id must be honored)", got, fixed)
}
// Nil cfg.ID still auto-generates a UUIDv7.
cfg2 := minimalHook(tenantID, hooks.EventPreToolUse)
cfg2.ID = uuid.Nil
auto, err := s.Create(ctx, cfg2)
if err != nil {
t.Fatalf("Create auto id: %v", err)
}
t.Cleanup(func() { s.Delete(masterCtx(), auto) })
if auto == uuid.Nil {
t.Fatal("Create returned nil id for cfg.ID=uuid.Nil path")
}
}
+7 -1
View File
@@ -45,7 +45,13 @@ func NewSQLiteHookStore(db *sql.DB) *SqliteHookStore {
// ─── Create ─────────────────────────────────────────────────────────────────
func (s *SqliteHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uuid.UUID, error) {
id := uuid.Must(uuid.NewV7())
// Honor caller-provided fixed ID (H9) — Phase 04 builtin seeder uses
// UUIDv5 for idempotent UPSERT; tests need deterministic IDs. Only
// auto-generate when the caller left ID unset.
id := cfg.ID
if id == uuid.Nil {
id = uuid.Must(uuid.NewV7())
}
now := time.Now().UTC().Format(time.RFC3339Nano)
cfgJSON, err := json.Marshal(cfg.Config)
+21 -8
View File
@@ -212,25 +212,38 @@ func TestSQLiteHookStore_TenantIsolation(t *testing.T) {
// ─── Partial unique indexes ───────────────────────────────────────────────────
func TestSQLiteHookStore_UniqueIndexPreventsScopedDuplicates(t *testing.T) {
// H9 (Phase 03): Create honors caller-supplied cfg.ID so the builtin seeder's
// idempotent UUIDv5 keys survive restarts and tests get deterministic IDs.
func TestSQLiteHookStore_CreateHonorsFixedID(t *testing.T) {
db := newHookTestDB(t)
tenantID, _ := seedHookTenantAgent(t, db)
s := NewSQLiteHookStore(db)
ctx := sqliteTenantCtx(tenantID)
fixed := uuid.MustParse("11111111-2222-3333-4444-555555555555")
cfg := sqliteMinimalHook(tenantID, hooks.EventUserPromptSubmit)
cfg.ID = fixed
cfg.Scope = hooks.ScopeTenant
id1, err := s.Create(ctx, cfg)
got, err := s.Create(ctx, cfg)
if err != nil {
t.Fatalf("first Create: %v", err)
t.Fatalf("Create: %v", err)
}
t.Cleanup(func() { s.Delete(sqliteMasterCtx(), got) })
if got != fixed {
t.Fatalf("Create returned id=%s, want %s (H9: caller id must be honored)", got, fixed)
}
t.Cleanup(func() { s.Delete(sqliteMasterCtx(), id1) })
// Same (tenant_id, event, handler_type) with ScopeTenant → should fail.
_, err = s.Create(ctx, cfg)
if err == nil {
t.Error("expected unique constraint error on duplicate scoped hook, got nil")
// A nil cfg.ID still auto-generates.
cfg2 := sqliteMinimalHook(tenantID, hooks.EventPreToolUse)
cfg2.ID = uuid.Nil
auto, err := s.Create(ctx, cfg2)
if err != nil {
t.Fatalf("Create auto: %v", err)
}
t.Cleanup(func() { s.Delete(sqliteMasterCtx(), auto) })
if auto == uuid.Nil {
t.Fatal("Create returned nil id for cfg.ID=uuid.Nil path")
}
}
+19 -1
View File
@@ -16,7 +16,7 @@ var schemaSQL string
// SchemaVersion is the current SQLite schema version.
// Bump this when adding new migration steps below.
const SchemaVersion = 20
const SchemaVersion = 21
// migrations maps version → SQL to apply when upgrading FROM that version.
// schema.sql always represents the LATEST full schema (for fresh DBs).
@@ -460,6 +460,14 @@ WHERE context_pruning IS NOT NULL
// Creates agent_hooks, hook_executions, tenant_hook_budget tables.
// All idempotent (CREATE TABLE IF NOT EXISTS / CREATE INDEX IF NOT EXISTS).
19: addAgentHooksTables,
// Version 20 → 21: relax agent_hooks CHECK constraints for script handler
// + builtin source. The inline patch is a no-op (`SELECT 1`) because SQLite
// can't ALTER a CHECK constraint — the table must be rebuilt, which
// requires `PRAGMA foreign_keys = OFF` outside any transaction. The actual
// rebuild runs in rebuildAgentHooksV21 triggered after the v20→21 migration
// tx commits (parallel to backfillV16 at EnsureSchema:677).
20: `SELECT 1;`,
}
// addAgentHooksTables is the SQLite incremental migration for schema v19 → v20.
@@ -679,6 +687,16 @@ func EnsureSchema(db *sql.DB) error {
return fmt.Errorf("v16 backfill: %w", err)
}
}
// v20 → v21: rebuild agent_hooks table to widen the
// handler_type + source CHECK constraints. SQLite forbids
// ALTER TABLE ... ADD/DROP CONSTRAINT, so the table is recreated
// with the new CHECKs. Runs outside the migration tx because
// PRAGMA foreign_keys is silently ignored inside a transaction.
if v == 20 {
if err := rebuildAgentHooksV21(context.Background(), db); err != nil {
return fmt.Errorf("v21 rebuild: %w", err)
}
}
slog.Info("sqlite: applied migration", "version", v+1)
}
}
+6 -15
View File
@@ -1570,9 +1570,12 @@ CREATE INDEX IF NOT EXISTS idx_vault_links_source
WHERE json_extract(metadata, '$.source') IS NOT NULL;
-- ============================================================
-- Table: agent_hooks (migration 000052)
-- Table: agent_hooks (migrations 000052 + 000053)
-- SQLite translation: JSONB→TEXT, TIMESTAMPTZ→TEXT, UUID→TEXT,
-- BYTEA→BLOB, DATE→TEXT (ISO8601), CHECK for enums.
-- v21 (migration 000053): handler_type adds 'script', source adds 'builtin',
-- old (event, handler_type) uniqueness indexes dropped — scripts routinely
-- want many small hooks per event.
-- ============================================================
CREATE TABLE IF NOT EXISTS agent_hooks (
@@ -1581,7 +1584,7 @@ CREATE TABLE IF NOT EXISTS agent_hooks (
agent_id TEXT REFERENCES agents(id) ON DELETE CASCADE,
scope TEXT NOT NULL CHECK (scope IN ('global', 'tenant', 'agent')),
event TEXT NOT NULL,
handler_type TEXT NOT NULL CHECK (handler_type IN ('command', 'http', 'prompt')),
handler_type TEXT NOT NULL CHECK (handler_type IN ('command', 'http', 'prompt', 'script')),
config TEXT NOT NULL DEFAULT '{}',
matcher TEXT,
if_expr TEXT,
@@ -1590,25 +1593,13 @@ CREATE TABLE IF NOT EXISTS agent_hooks (
priority INTEGER NOT NULL DEFAULT 0,
enabled INTEGER NOT NULL DEFAULT 1,
version INTEGER NOT NULL DEFAULT 1,
source TEXT NOT NULL DEFAULT 'ui' CHECK (source IN ('ui', 'api', 'seed')),
source TEXT NOT NULL DEFAULT 'ui' CHECK (source IN ('ui', 'api', 'seed', 'builtin')),
metadata TEXT NOT NULL DEFAULT '{}',
created_by TEXT,
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))
);
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_global
ON agent_hooks (event, handler_type)
WHERE scope = 'global';
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_tenant
ON agent_hooks (tenant_id, event, handler_type)
WHERE scope = 'tenant';
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_agent
ON agent_hooks (tenant_id, agent_id, event, handler_type)
WHERE scope = 'agent';
CREATE INDEX IF NOT EXISTS idx_hooks_lookup
ON agent_hooks (tenant_id, agent_id, event)
WHERE enabled = 1;
@@ -0,0 +1,84 @@
//go:build sqlite || sqliteonly
package sqlitestore
import (
"context"
"database/sql"
"fmt"
)
// rebuildAgentHooksV21 recreates the agent_hooks table with relaxed CHECK
// constraints — handler_type accepts `script` and source accepts `builtin`.
// Uniqueness indexes on (event, handler_type) per scope are dropped because
// a tenant may want many small script hooks per event (e.g. one redactor per
// PII class).
//
// SQLite cannot ALTER a CHECK constraint: the table must be dropped and
// recreated, then data copied. PRAGMA foreign_keys is a no-op inside a
// transaction, so the sequence runs OFF outside any tx, then wraps the rename
// + recreate + copy in one transaction for atomicity:
//
// 1. PRAGMA foreign_keys = OFF (outside tx)
// 2. BEGIN
// 3. ALTER TABLE agent_hooks RENAME TO agent_hooks_old
// 4. CREATE TABLE agent_hooks (...new CHECKs)
// 5. INSERT INTO agent_hooks SELECT * FROM agent_hooks_old
// 6. DROP TABLE agent_hooks_old
// 7. recreate the post-rename index that survives the rebuild
// 8. COMMIT
// 9. PRAGMA foreign_keys = ON
//
// Runs after the v20→v21 migration tx commits (parallel to backfillV16 at
// EnsureSchema:677) so the caller's outer migration loop sees v21 only when
// the rebuild also succeeded.
func rebuildAgentHooksV21(ctx context.Context, db *sql.DB) error {
if _, err := db.ExecContext(ctx, "PRAGMA foreign_keys = OFF"); err != nil {
return fmt.Errorf("v21 fk off: %w", err)
}
defer func() { _, _ = db.ExecContext(ctx, "PRAGMA foreign_keys = ON") }()
tx, err := db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("v21 begin tx: %w", err)
}
steps := []string{
`ALTER TABLE agent_hooks RENAME TO agent_hooks_old`,
`CREATE TABLE agent_hooks (
id TEXT NOT NULL PRIMARY KEY,
tenant_id TEXT NOT NULL DEFAULT '0193a5b0-7000-7000-8000-000000000001',
agent_id TEXT REFERENCES agents(id) ON DELETE CASCADE,
scope TEXT NOT NULL CHECK (scope IN ('global', 'tenant', 'agent')),
event TEXT NOT NULL,
handler_type TEXT NOT NULL CHECK (handler_type IN ('command', 'http', 'prompt', 'script')),
config TEXT NOT NULL DEFAULT '{}',
matcher TEXT,
if_expr TEXT,
timeout_ms INTEGER NOT NULL DEFAULT 5000,
on_timeout TEXT NOT NULL DEFAULT 'block' CHECK (on_timeout IN ('block', 'allow')),
priority INTEGER NOT NULL DEFAULT 0,
enabled INTEGER NOT NULL DEFAULT 1,
version INTEGER NOT NULL DEFAULT 1,
source TEXT NOT NULL DEFAULT 'ui' CHECK (source IN ('ui', 'api', 'seed', 'builtin')),
metadata TEXT NOT NULL DEFAULT '{}',
created_by TEXT,
created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')),
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))
)`,
`INSERT INTO agent_hooks SELECT * FROM agent_hooks_old`,
`DROP TABLE agent_hooks_old`,
`CREATE INDEX IF NOT EXISTS idx_agent_hooks_lookup ON agent_hooks (tenant_id, agent_id, event) WHERE enabled = 1`,
}
for _, s := range steps {
if _, err := tx.ExecContext(ctx, s); err != nil {
_ = tx.Rollback()
head := s
if len(head) > 60 {
head = head[:60]
}
return fmt.Errorf("v21 step failed (%q): %w", head, err)
}
}
return tx.Commit()
}
+4 -2
View File
@@ -172,7 +172,7 @@ func (t *DelegateTool) Execute(ctx context.Context, args map[string]any) *Result
HookEvent: hooks.EventSubagentStart,
Depth: hooks.DepthFrom(ctx),
}
dec, err := t.hookDispatcher.Fire(ctx, evt)
r, err := t.hookDispatcher.Fire(ctx, evt)
if err != nil {
t.emitEvent(ctx, eventbus.EventDelegateFailed, eventbus.DelegateFailedPayload{
DelegationID: req.DelegationID,
@@ -182,7 +182,9 @@ func (t *DelegateTool) Execute(ctx context.Context, args map[string]any) *Result
})
return ErrorResult(fmt.Sprintf("subagent_start hook error: %v", err))
}
if dec == hooks.DecisionBlock {
// Updated* from FireResult intentionally unused — delegate has no
// mutation need in Wave 1.
if r.Decision == hooks.DecisionBlock {
t.emitEvent(ctx, eventbus.EventDelegateFailed, eventbus.DelegateFailedPayload{
DelegationID: req.DelegationID,
FromAgent: req.FromAgentKey,
+2 -2
View File
@@ -16,9 +16,9 @@ type fakeDispatcher struct {
calls int
}
func (f *fakeDispatcher) Fire(_ context.Context, _ hooks.Event) (hooks.Decision, error) {
func (f *fakeDispatcher) Fire(_ context.Context, _ hooks.Event) (hooks.FireResult, error) {
f.calls++
return f.decision, nil
return hooks.FireResult{Decision: f.decision}, nil
}
// --- minimal noop stores to satisfy AgentLinkStore / AgentCRUDStore ---
+1 -1
View File
@@ -2,4 +2,4 @@ package upgrade
// RequiredSchemaVersion is the schema migration version this binary requires.
// Bump this whenever adding a new SQL migration file.
const RequiredSchemaVersion uint = 52
const RequiredSchemaVersion uint = 53
@@ -0,0 +1,21 @@
-- 000053 down — restore the pre-script constraints. Any rows with the new
-- values are deleted first so the restored CHECK won't reject valid historical
-- rows. Uniqueness indexes are recreated in their original form.
DELETE FROM agent_hooks WHERE handler_type = 'script' OR source = 'builtin';
ALTER TABLE agent_hooks DROP CONSTRAINT IF EXISTS agent_hooks_handler_type_check;
ALTER TABLE agent_hooks ADD CONSTRAINT agent_hooks_handler_type_check
CHECK (handler_type IN ('command', 'http', 'prompt'));
ALTER TABLE agent_hooks DROP CONSTRAINT IF EXISTS agent_hooks_source_check;
ALTER TABLE agent_hooks ADD CONSTRAINT agent_hooks_source_check
CHECK (source IN ('ui', 'api', 'seed'));
ALTER TABLE agent_hooks ALTER COLUMN source TYPE VARCHAR(8);
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_global
ON agent_hooks (event, handler_type) WHERE scope = 'global';
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_tenant
ON agent_hooks (tenant_id, event, handler_type) WHERE scope = 'tenant';
CREATE UNIQUE INDEX IF NOT EXISTS uq_hooks_agent
ON agent_hooks (tenant_id, agent_id, event, handler_type) WHERE scope = 'agent';
@@ -0,0 +1,21 @@
-- 000053 — Relax CHECK constraints for script handler + builtin source.
--
-- Phase 03 of the Wave 1 hooks plan. Before this migration the hook system
-- only accepts command/http/prompt handlers and ui/api/seed sources. Phase 02
-- landed a goja-backed script handler; Phase 04 ships builtin hooks whose
-- source marker must be persisted as 'builtin'. Uniqueness indexes on
-- (event, handler_type) per scope are also dropped — scripts routinely want
-- many small hooks per event (e.g. a redactor per PII type).
ALTER TABLE agent_hooks DROP CONSTRAINT IF EXISTS agent_hooks_handler_type_check;
ALTER TABLE agent_hooks ADD CONSTRAINT agent_hooks_handler_type_check
CHECK (handler_type IN ('command', 'http', 'prompt', 'script'));
ALTER TABLE agent_hooks ALTER COLUMN source TYPE VARCHAR(16);
ALTER TABLE agent_hooks DROP CONSTRAINT IF EXISTS agent_hooks_source_check;
ALTER TABLE agent_hooks ADD CONSTRAINT agent_hooks_source_check
CHECK (source IN ('ui', 'api', 'seed', 'builtin'));
DROP INDEX IF EXISTS uq_hooks_global;
DROP INDEX IF EXISTS uq_hooks_tenant;
DROP INDEX IF EXISTS uq_hooks_agent;
+12 -12
View File
@@ -56,13 +56,13 @@ func TestHooksChaos_HTTPHandler_ProviderDown(t *testing.T) {
d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{
hooks.HandlerHTTP: &hookhandlers.HTTPHandler{Client: &http.Client{Timeout: 1 * time.Second}},
})
dec, _ := d.Fire(ctx, hooks.Event{
r, _ := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
// Blocking event + unexpected error → fail-closed Block.
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on provider down)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on provider down)", r.Decision)
}
}
@@ -104,14 +104,14 @@ func TestHooksChaos_PerHookTimeout(t *testing.T) {
hooks.HandlerHTTP: &hookhandlers.HTTPHandler{Client: srv.Client()},
})
start := time.Now()
dec, _ := d.Fire(ctx, hooks.Event{
r, _ := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
elapsed := time.Since(start)
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (OnTimeout=block)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (OnTimeout=block)", r.Decision)
}
// Must NOT have waited the full 2s; per-hook timeout caps at ~300ms+retry.
if elapsed > 4*time.Second {
@@ -193,15 +193,15 @@ func TestHooksChaos_LoopDepthExceeded(t *testing.T) {
// Wrap ctx with depth > MaxLoopDepth.
ctx := hooks.WithDepth(tenantCtx(tenantID), hooks.MaxLoopDepth+1)
dec, err := d.Fire(ctx, hooks.Event{
r, err := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
if !errors.Is(err, hooks.ErrLoopDepthExceeded) {
t.Errorf("err=%v, want ErrLoopDepthExceeded", err)
}
if dec != hooks.DecisionError {
t.Errorf("decision=%q, want error", dec)
if r.Decision != hooks.DecisionError {
t.Errorf("decision=%q, want error", r.Decision)
}
}
@@ -295,13 +295,13 @@ func TestHooksChaos_HTTPHandler_5xxRetriesThenErrors(t *testing.T) {
d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{
hooks.HandlerHTTP: &hookhandlers.HTTPHandler{Client: srv.Client()},
})
dec, _ := d.Fire(ctx, hooks.Event{
r, _ := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
// Blocking event + repeated 5xx → fail-closed Block.
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on 5xx)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (fail-closed on 5xx)", r.Decision)
}
// Verify the handler did perform its single retry (hits >= 2).
if hits.Load() < 2 {
+12 -12
View File
@@ -73,14 +73,14 @@ func TestHooksE2E_AllEvents_AllowsDefault(t *testing.T) {
db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID)
})
dec, err := d.Fire(ctx, hooks.Event{
r, err := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID, HookEvent: ev,
})
if err != nil {
t.Fatalf("Fire %s: %v", ev, err)
}
if dec != hooks.DecisionAllow {
t.Errorf("%s: decision=%q, want allow", ev, dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("%s: decision=%q, want allow", ev, r.Decision)
}
// Non-blocking events write audit from a goroutine; poll briefly
@@ -122,15 +122,15 @@ func TestHooksE2E_CommandHandler_BlockOnExitTwo(t *testing.T) {
hooks.HandlerCommand: &hookhandlers.CommandHandler{Edition: edition.Lite},
})
dec, err := d.Fire(ctx, hooks.Event{
r, err := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (exit 2)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (exit 2)", r.Decision)
}
assertHookAuditCount(t, db, hookID, "block", 1)
}
@@ -174,12 +174,12 @@ func TestHooksE2E_ContextUpdateInjection(t *testing.T) {
d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{
hooks.HandlerHTTP: &hookhandlers.HTTPHandler{Client: srv.Client()},
})
dec, err := d.Fire(ctx, hooks.Event{
r, err := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventUserPromptSubmit,
})
if err != nil || dec != hooks.DecisionAllow {
t.Errorf("decision=%q err=%v, want allow+nil", dec, err)
if err != nil || r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q err=%v, want allow+nil", r.Decision, err)
}
assertHookAuditCount(t, db, hookID, "allow", 1)
}
@@ -276,13 +276,13 @@ func TestHooksE2E_EditionGate_CommandOnStandard(t *testing.T) {
d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{
hooks.HandlerCommand: &hookhandlers.CommandHandler{Edition: edition.Standard},
})
dec, _ := d.Fire(ctx, hooks.Event{
r, _ := d.Fire(ctx, hooks.Event{
EventID: uuid.NewString(), TenantID: tenantID, AgentID: agentID,
HookEvent: hooks.EventPreToolUse,
})
// Blocking event + handler error → fail-closed Block.
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (edition gate)", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block (edition gate)", r.Decision)
}
}
+9 -9
View File
@@ -88,12 +88,12 @@ func TestHooksIntegration_HTTPHandler_AllowWritesAudit(t *testing.T) {
AgentID: agentID,
HookEvent: hooks.EventUserPromptSubmit,
}
dec, err := d.Fire(ctx, ev)
r, err := d.Fire(ctx, ev)
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", r.Decision)
}
// Give the audit writer a moment (sync path, but defensive).
@@ -159,12 +159,12 @@ func TestHooksIntegration_HTTPHandler_Block(t *testing.T) {
AgentID: agentID,
HookEvent: hooks.EventUserPromptSubmit,
}
dec, err := d.Fire(ctx, ev)
r, err := d.Fire(ctx, ev)
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionBlock {
t.Errorf("decision=%q, want block", dec)
if r.Decision != hooks.DecisionBlock {
t.Errorf("decision=%q, want block", r.Decision)
}
var count int
@@ -221,12 +221,12 @@ func TestHooksIntegration_CommandHandler_LiteEdition(t *testing.T) {
AgentID: agentID,
HookEvent: hooks.EventUserPromptSubmit,
}
dec, err := d.Fire(ctx, ev)
r, err := d.Fire(ctx, ev)
if err != nil {
t.Fatalf("Fire: %v", err)
}
if dec != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", dec)
if r.Decision != hooks.DecisionAllow {
t.Errorf("decision=%q, want allow", r.Decision)
}
var count int