From ee328d4266e2898ff79f89113caf7d3281f9145a Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 16 Apr 2026 10:30:05 +0700 Subject: [PATCH] refactor: consolidate hooks table migration and drop deprecated agent_id column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Consolidated PG migration 054: agent_hooks rename + junction table + deprecated column drop - Updated SQLite schema v19 with final consolidated migration - Removed obsolete schema rebuild files (v21, v23) - Updated Go store layer (pg/hooks.go, sqlitestore/hooks.go) - Updated integration tests to use new table names (hooks, hook_agents) - Updated TypeScript protocol, UI components, and i18n strings - Updated gateway methods to reflect schema changes Tables: agent_hooks → hooks, agent_hook_agents → hook_agents --- cmd/gateway_agents.go | 3 + internal/channels/pancake/api_client.go | 2 +- internal/channels/pancake/comment_handler.go | 3 +- internal/channels/pancake/pancake.go | 1 - internal/gateway/methods/config.go | 5 +- internal/gateway/methods/config_defaults.go | 6 +- internal/gateway/methods/hooks.go | 17 ++ internal/gateway/methods/hooks_test.go | 4 + internal/hooks/builtin/seed_test.go | 4 + internal/hooks/config.go | 8 +- internal/hooks/dispatcher_test.go | 4 + .../migration_command_autodisable_test.go | 4 + internal/hooks/store.go | 6 + internal/hooks/types.go | 3 +- internal/store/base/tables.go | 2 +- internal/store/pg/hooks.go | 152 ++++++++++++++--- internal/store/pg/hooks_test.go | 4 +- internal/store/sqlitestore/hooks.go | 160 ++++++++++++++---- internal/store/sqlitestore/schema.go | 70 +++----- internal/store/sqlitestore/schema.sql | 25 ++- .../schema_rebuild_agent_hooks_v21.go | 84 --------- migrations/000054_agent_hooks_name.down.sql | 17 +- migrations/000054_agent_hooks_name.up.sql | 41 ++++- tests/integration/hooks_builtin_seed_test.go | 10 +- tests/integration/hooks_chaos_test.go | 12 +- tests/integration/hooks_e2e_test.go | 10 +- tests/integration/hooks_phase08_helpers.go | 6 +- tests/integration/hooks_pipeline_test.go | 6 +- tests/integration/hooks_rbac_test.go | 10 +- .../hooks_sqlite_v21_rebuild_test.go | 141 --------------- tests/integration/hooks_tracing_test.go | 2 +- tests/integration/v3_hooks_store_test.go | 6 +- ui/web/src/api/protocol.ts | 1 + ui/web/src/hooks/use-hooks.ts | 1 + ui/web/src/i18n/locales/en/agents.json | 3 +- ui/web/src/i18n/locales/en/hooks.json | 2 + ui/web/src/i18n/locales/vi/agents.json | 3 +- ui/web/src/i18n/locales/vi/hooks.json | 2 + ui/web/src/i18n/locales/zh/agents.json | 3 +- ui/web/src/i18n/locales/zh/hooks.json | 2 + ui/web/src/lib/query-keys.ts | 1 + .../context-pruning-section.tsx | 16 +- .../config-sections/subagents-section.tsx | 23 ++- .../pages/config/hooks/use-config-defaults.ts | 81 +++++++++ .../hooks/components/hook-form-dialog.tsx | 35 ++++ .../pages/hooks/components/hook-list-row.tsx | 5 + ui/web/src/pages/hooks/hooks-page.tsx | 2 + ui/web/src/schemas/hooks.schema.ts | 1 + ui/web/src/types/agent.ts | 1 + 49 files changed, 622 insertions(+), 388 deletions(-) delete mode 100644 internal/store/sqlitestore/schema_rebuild_agent_hooks_v21.go delete mode 100644 tests/integration/hooks_sqlite_v21_rebuild_test.go create mode 100644 ui/web/src/pages/config/hooks/use-config-defaults.ts diff --git a/cmd/gateway_agents.go b/cmd/gateway_agents.go index 232e456f..e2cb91bf 100644 --- a/cmd/gateway_agents.go +++ b/cmd/gateway_agents.go @@ -189,6 +189,9 @@ func setupSubagents(providerReg *providers.Registry, cfg *config.Config, msgBus if sc.ArchiveAfterMinutes > 0 { subCfg.ArchiveAfterMinutes = sc.ArchiveAfterMinutes } + if sc.MaxRetries > 0 { + subCfg.MaxRetries = sc.MaxRetries + } if sc.Model != "" { subCfg.Model = sc.Model } diff --git a/internal/channels/pancake/api_client.go b/internal/channels/pancake/api_client.go index 6f37e230..42ba502e 100644 --- a/internal/channels/pancake/api_client.go +++ b/internal/channels/pancake/api_client.go @@ -294,7 +294,7 @@ func (c *APIClient) ReactComment(ctx context.Context, conversationID, messageID if conversationID == "" || messageID == "" { return fmt.Errorf("pancake: react-comment requires conversation_id and message_id") } - if strings.ContainsAny(conversationID, "/?#") || strings.ContainsAny(messageID, "/?#") { + if strings.ContainsAny(conversationID, "/?#\\") || strings.ContainsAny(messageID, "/?#\\") { return fmt.Errorf("pancake: invalid character in conversation_id or message_id") } diff --git a/internal/channels/pancake/comment_handler.go b/internal/channels/pancake/comment_handler.go index 175a8154..a6c3d6f3 100644 --- a/internal/channels/pancake/comment_handler.go +++ b/internal/channels/pancake/comment_handler.go @@ -60,8 +60,7 @@ func (ch *Channel) handleCommentEvent(data MessagingData) { // Auto-react BEFORE keyword filter — fires on all valid non-duplicate comments. // Independent of comment_reply: reacts even if reply is disabled. - if ch.config.Features.AutoReact && ch.platform == "facebook" && - data.ConversationID != "" && data.Message.ID != "" { + if ch.config.Features.AutoReact && ch.platform == "facebook" && data.Message.ID != "" { select { case ch.reactSem <- struct{}{}: go ch.reactCommentAsync(data.ConversationID, data.Message.ID) diff --git a/internal/channels/pancake/pancake.go b/internal/channels/pancake/pancake.go index a29208ed..9711936f 100644 --- a/internal/channels/pancake/pancake.go +++ b/internal/channels/pancake/pancake.go @@ -157,7 +157,6 @@ func (ch *Channel) Start(ctx context.Context) error { "note", "webhook_secret not configured; incoming webhook requests will not be authenticated") } - // [F9] Warn if auto_react enabled without webhook signature verification. // Without HMAC, any actor reaching the webhook endpoint can trigger Pancake API calls. if ch.config.Features.AutoReact && ch.webhookSecret == "" { slog.Warn("security.pancake_auto_react_without_hmac: auto_react is enabled but "+ diff --git a/internal/gateway/methods/config.go b/internal/gateway/methods/config.go index 10da6070..590d66a8 100644 --- a/internal/gateway/methods/config.go +++ b/internal/gateway/methods/config.go @@ -40,7 +40,10 @@ func (m *ConfigMethods) Register(router *gateway.MethodRouter) { router.Register(protocol.MethodConfigApply, m.requireMasterScope(m.requireOwner(m.handleApply))) router.Register(protocol.MethodConfigPatch, m.requireMasterScope(m.requireOwner(m.handlePatch))) router.Register(protocol.MethodConfigSchema, m.requireMasterScope(m.requireOwner(m.handleSchema))) - router.Register(protocol.MethodConfigDefaults, m.requireMasterScope(m.requireOwner(m.handleDefaults))) + // config.defaults is read-only + secret-free (Go consts + agents.defaults overlay), + // so it only needs requireMasterScope — owner gating would spam auth errors for + // operators viewing agent detail pages. + router.Register(protocol.MethodConfigDefaults, m.requireMasterScope(m.handleDefaults)) } // requireOwner wraps a handler to only allow owner-role users. diff --git a/internal/gateway/methods/config_defaults.go b/internal/gateway/methods/config_defaults.go index 11c2e3f9..686d7850 100644 --- a/internal/gateway/methods/config_defaults.go +++ b/internal/gateway/methods/config_defaults.go @@ -146,11 +146,13 @@ func overlaySubagents(dst *subagentDefaultsJSON, src *config.SubagentsConfig) { if src.MaxConcurrent > 0 { dst.MaxConcurrent = src.MaxConcurrent } + // Mirror runtime clamps in cmd/gateway_agents.go so UI placeholders reflect + // the effective value (not operator's raw over-limit config.json). if src.MaxSpawnDepth > 0 { - dst.MaxSpawnDepth = src.MaxSpawnDepth + dst.MaxSpawnDepth = min(src.MaxSpawnDepth, 5) } if src.MaxChildrenPerAgent > 0 { - dst.MaxChildrenPerAgent = src.MaxChildrenPerAgent + dst.MaxChildrenPerAgent = min(src.MaxChildrenPerAgent, 20) } if src.ArchiveAfterMinutes > 0 { dst.ArchiveAfterMinutes = src.ArchiveAfterMinutes diff --git a/internal/gateway/methods/hooks.go b/internal/gateway/methods/hooks.go index 9642cee1..2fe59783 100644 --- a/internal/gateway/methods/hooks.go +++ b/internal/gateway/methods/hooks.go @@ -242,6 +242,19 @@ func applyHookPatch(cur hooks.HookConfig, p map[string]any) hooks.HookConfig { if v, ok := p["name"].(string); ok { cur.Name = v } + if v, ok := p["agent_ids"]; ok { + if arr, ok := v.([]any); ok { + var ids []uuid.UUID + for _, item := range arr { + if s, ok := item.(string); ok { + if id, err := uuid.Parse(s); err == nil { + ids = append(ids, id) + } + } + } + cur.AgentIDs = ids + } + } if v, ok := p["event"].(string); ok && v != "" { cur.Event = hooks.HookEvent(v) } @@ -421,6 +434,10 @@ func parseHookConfigParams(raw json.RawMessage) (*hooks.HookConfig, error) { cfg.ID = uuid.Nil cfg.CreatedBy = nil cfg.Version = 0 + // Backward compat: singular agent_id → single-element AgentIDs. + if len(cfg.AgentIDs) == 0 && cfg.AgentID != nil && *cfg.AgentID != uuid.Nil { + cfg.AgentIDs = []uuid.UUID{*cfg.AgentID} + } return &cfg, nil } diff --git a/internal/gateway/methods/hooks_test.go b/internal/gateway/methods/hooks_test.go index 1b113d86..bacb213f 100644 --- a/internal/gateway/methods/hooks_test.go +++ b/internal/gateway/methods/hooks_test.go @@ -78,6 +78,10 @@ func (f *fakeStore) ResolveForEvent(_ context.Context, _ hooks.Event) ([]hooks.H func (f *fakeStore) WriteExecution(_ context.Context, _ hooks.HookExecution) error { return nil } +func (f *fakeStore) SetHookAgents(_ context.Context, _ uuid.UUID, _ []uuid.UUID) error { return nil } +func (f *fakeStore) GetHookAgents(_ context.Context, _ uuid.UUID) ([]uuid.UUID, error) { + return nil, nil +} // Basic happy-path tests exercise the parse helpers and the configuration // invariants. Full RPC wiring (gateway.Client, MethodRouter) is covered in diff --git a/internal/hooks/builtin/seed_test.go b/internal/hooks/builtin/seed_test.go index 3b52b0d4..60e25dae 100644 --- a/internal/hooks/builtin/seed_test.go +++ b/internal/hooks/builtin/seed_test.go @@ -97,6 +97,10 @@ func (f *fakeHookStore) ResolveForEvent(context.Context, hooks.Event) ([]hooks.H } func (f *fakeHookStore) WriteExecution(context.Context, hooks.HookExecution) error { return nil } +func (f *fakeHookStore) SetHookAgents(context.Context, uuid.UUID, []uuid.UUID) error { return nil } +func (f *fakeHookStore) GetHookAgents(context.Context, uuid.UUID) ([]uuid.UUID, error) { + return nil, nil +} type errStr string diff --git a/internal/hooks/config.go b/internal/hooks/config.go index 187de8e9..64a6a8b1 100644 --- a/internal/hooks/config.go +++ b/internal/hooks/config.go @@ -96,8 +96,12 @@ func (h *HookConfig) validateScope() error { if h.TenantID == SentinelTenantID || h.TenantID == uuid.Nil { return fmt.Errorf("hook: agent scope requires a real tenant_id") } - if h.AgentID == nil || *h.AgentID == uuid.Nil { - return fmt.Errorf("hook: agent scope requires agent_id") + if len(h.AgentIDs) == 0 { + if h.AgentID != nil && *h.AgentID != uuid.Nil { + h.AgentIDs = []uuid.UUID{*h.AgentID} + } else { + return fmt.Errorf("hook: agent scope requires at least one agent_id") + } } default: return fmt.Errorf("hook: unknown scope %q", h.Scope) diff --git a/internal/hooks/dispatcher_test.go b/internal/hooks/dispatcher_test.go index 7c7f673a..8d2eff42 100644 --- a/internal/hooks/dispatcher_test.go +++ b/internal/hooks/dispatcher_test.go @@ -65,6 +65,10 @@ func (f *fakeStore) WriteExecution(_ context.Context, e hooks.HookExecution) err return nil } +func (f *fakeStore) SetHookAgents(context.Context, uuid.UUID, []uuid.UUID) error { return nil } +func (f *fakeStore) GetHookAgents(context.Context, uuid.UUID) ([]uuid.UUID, error) { + return nil, nil +} func (f *fakeStore) snapshotExecs() []hooks.HookExecution { f.mu.Lock() defer f.mu.Unlock() diff --git a/internal/hooks/migration_command_autodisable_test.go b/internal/hooks/migration_command_autodisable_test.go index a62dd704..596a7fa4 100644 --- a/internal/hooks/migration_command_autodisable_test.go +++ b/internal/hooks/migration_command_autodisable_test.go @@ -61,6 +61,10 @@ func (m *migStore) ResolveForEvent(context.Context, hooks.Event) ([]hooks.HookCo return nil, nil } func (m *migStore) WriteExecution(context.Context, hooks.HookExecution) error { return nil } +func (m *migStore) SetHookAgents(context.Context, uuid.UUID, []uuid.UUID) error { return nil } +func (m *migStore) GetHookAgents(context.Context, uuid.UUID) ([]uuid.UUID, error) { + return nil, nil +} // fixtureRows covers every branch of the migration: // diff --git a/internal/hooks/store.go b/internal/hooks/store.go index 7d067967..744db0e5 100644 --- a/internal/hooks/store.go +++ b/internal/hooks/store.go @@ -76,4 +76,10 @@ type HookStore interface { // WriteExecution appends an immutable execution audit row. // Caller must pre-truncate Error to 256 chars and encrypt ErrorDetail. WriteExecution(ctx context.Context, exec HookExecution) error + + // SetHookAgents replaces all junction rows for hookID with the given agentIDs. + SetHookAgents(ctx context.Context, hookID uuid.UUID, agentIDs []uuid.UUID) error + + // GetHookAgents returns the agent UUIDs linked to hookID via the junction table. + GetHookAgents(ctx context.Context, hookID uuid.UUID) ([]uuid.UUID, error) } diff --git a/internal/hooks/types.go b/internal/hooks/types.go index 98ca67e7..0c53c3ad 100644 --- a/internal/hooks/types.go +++ b/internal/hooks/types.go @@ -160,7 +160,8 @@ type FireResult struct { type HookConfig struct { ID uuid.UUID `json:"id"` TenantID uuid.UUID `json:"tenant_id"` - AgentID *uuid.UUID `json:"agent_id,omitempty"` + AgentID *uuid.UUID `json:"agent_id,omitempty"` // DEPRECATED: kept for JSON backward compat + AgentIDs []uuid.UUID `json:"agent_ids,omitempty"` Event HookEvent `json:"event"` HandlerType HandlerType `json:"handler_type"` Scope Scope `json:"scope"` diff --git a/internal/store/base/tables.go b/internal/store/base/tables.go index ba8195e6..a4f9a581 100644 --- a/internal/store/base/tables.go +++ b/internal/store/base/tables.go @@ -18,7 +18,7 @@ var TablesWithUpdatedAt = map[string]bool{ "memory_documents": true, "memory_chunks": true, "embedding_cache": true, "vault_documents": true, "secure_cli_binaries": true, "tenants": true, - "agent_hooks": true, + "hooks": true, } // TableHasUpdatedAt returns true if the table has an updated_at column. diff --git a/internal/store/pg/hooks.go b/internal/store/pg/hooks.go index ce8da980..b3c5fcae 100644 --- a/internal/store/pg/hooks.go +++ b/internal/store/pg/hooks.go @@ -79,13 +79,13 @@ func (s *PGHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uuid.UU } _, err = s.db.ExecContext(ctx, ` - INSERT INTO agent_hooks - (id, tenant_id, agent_id, scope, event, handler_type, + INSERT INTO hooks + (id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at) - VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,$13,1,$14,$15,$16,$17,$18,$18)`, - id, tid, cfg.AgentID, string(cfg.Scope), string(cfg.Event), string(cfg.HandlerType), + VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11,$12,1,$13,$14,$15,$16,$17,$17)`, + id, tid, string(cfg.Scope), string(cfg.Event), string(cfg.HandlerType), cfgJSON, matcher, ifExpr, cfg.TimeoutMS, string(cfg.OnTimeout), cfg.Priority, cfg.Enabled, string(cfg.Source), metaJSON, name, cfg.CreatedBy, now, @@ -93,6 +93,18 @@ func (s *PGHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uuid.UU if err != nil { return uuid.Nil, fmt.Errorf("insert hook: %w", err) } + // Bridge deprecated AgentID → AgentIDs for junction insertion. + agentIDs := cfg.AgentIDs + if len(agentIDs) == 0 && cfg.AgentID != nil && *cfg.AgentID != uuid.Nil { + agentIDs = []uuid.UUID{*cfg.AgentID} + } + for _, aid := range agentIDs { + if _, err := s.db.ExecContext(ctx, + `INSERT INTO hook_agents (hook_id, agent_id) VALUES ($1, $2) ON CONFLICT DO NOTHING`, + id, aid); err != nil { + return uuid.Nil, fmt.Errorf("insert hook agent link: %w", err) + } + } s.invalidateCache() return id, nil } @@ -101,11 +113,11 @@ func (s *PGHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uuid.UU func (s *PGHookStore) GetByID(ctx context.Context, id uuid.UUID) (*hooks.HookConfig, error) { q := ` - SELECT id, tenant_id, agent_id, scope, event, handler_type, + SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at - FROM agent_hooks WHERE id = $1` + FROM hooks WHERE id = $1` args := []any{id} // Tenant-scope guard: non-master callers only see their own rows + globals. @@ -127,16 +139,19 @@ func (s *PGHookStore) GetByID(ctx context.Context, id uuid.UUID) (*hooks.HookCon if err != nil { return nil, fmt.Errorf("get hook by id: %w", err) } + if aids, err := s.GetHookAgents(ctx, cfg.ID); err == nil { + cfg.AgentIDs = aids + } return cfg, nil } // ─── List ──────────────────────────────────────────────────────────────────── func (s *PGHookStore) List(ctx context.Context, filter hooks.ListFilter) ([]hooks.HookConfig, error) { - q := `SELECT id, tenant_id, agent_id, scope, event, handler_type, + q := `SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, - created_at, updated_at FROM agent_hooks WHERE 1=1` + created_at, updated_at FROM hooks WHERE 1=1` var args []any n := 1 @@ -156,7 +171,7 @@ func (s *PGHookStore) List(ctx context.Context, filter hooks.ListFilter) ([]hook } if filter.AgentID != nil { - q += fmt.Sprintf(" AND agent_id = $%d", n) + q += fmt.Sprintf(" AND id IN (SELECT hook_id FROM hook_agents WHERE agent_id = $%d)", n) args = append(args, *filter.AgentID) n++ } @@ -192,7 +207,16 @@ func (s *PGHookStore) List(ctx context.Context, filter hooks.ListFilter) ([]hook } result = append(result, *cfg) } - return result, rows.Err() + if err := rows.Err(); err != nil { + return nil, err + } + // Batch-populate AgentIDs from junction table. + for i := range result { + if aids, err := s.GetHookAgents(ctx, result[i].ID); err == nil { + result[i].AgentIDs = aids + } + } + return result, nil } // ─── Update ────────────────────────────────────────────────────────────────── @@ -221,6 +245,18 @@ func (s *PGHookStore) Update(ctx context.Context, id uuid.UUID, updates map[stri } } + // Handle agent_ids separately — routes to junction table, not a column. + if raw, ok := updates["agent_ids"]; ok { + delete(updates, "agent_ids") + ids, err := parseAgentIDsFromAny(raw) + if err != nil { + return fmt.Errorf("invalid agent_ids: %w", err) + } + if err := s.SetHookAgents(ctx, id, ids); err != nil { + return err + } + } + // Marshal map/slice values to JSON for JSONB columns. for k, v := range updates { switch k { @@ -251,7 +287,7 @@ func (s *PGHookStore) Update(ctx context.Context, id uuid.UUID, updates map[stri i++ args = append(args, id) - q := fmt.Sprintf("UPDATE agent_hooks SET %s WHERE id = $%d", + q := fmt.Sprintf("UPDATE hooks SET %s WHERE id = $%d", strings.Join(setClauses, ", "), i) i++ @@ -290,7 +326,7 @@ func (s *PGHookStore) Delete(ctx context.Context, id uuid.UUID) error { } } - q := "DELETE FROM agent_hooks WHERE id = $1" + q := "DELETE FROM hooks WHERE id = $1" args := []any{id} if !store.IsMasterScope(ctx) { @@ -323,10 +359,16 @@ func (s *PGHookStore) ResolveForEvent(ctx context.Context, event hooks.Event) ([ // Check max version in DB to validate cache freshness. var maxVersion int err := s.db.QueryRowContext(ctx, - `SELECT COALESCE(MAX(version),0) FROM agent_hooks + `SELECT COALESCE(MAX(version),0) FROM hooks WHERE enabled = TRUE AND event = $1 AND (tenant_id = $2 OR tenant_id = $3) - AND (agent_id = $4 OR agent_id IS NULL)`, + AND ( + scope IN ('global', 'tenant') + OR (scope = 'agent' AND EXISTS ( + SELECT 1 FROM hook_agents aha + WHERE aha.hook_id = hooks.id AND aha.agent_id = $4 + )) + )`, string(hookEvent), tenantID, store.MasterTenantID, agentID, ).Scan(&maxVersion) if err != nil { @@ -343,14 +385,20 @@ func (s *PGHookStore) ResolveForEvent(ctx context.Context, event hooks.Event) ([ } rows, err := s.db.QueryContext(ctx, ` - SELECT id, tenant_id, agent_id, scope, event, handler_type, + SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at - FROM agent_hooks + FROM hooks WHERE enabled = TRUE AND event = $1 AND (tenant_id = $2 OR tenant_id = $3) - AND (agent_id = $4 OR agent_id IS NULL) + AND ( + scope IN ('global', 'tenant') + OR (scope = 'agent' AND EXISTS ( + SELECT 1 FROM hook_agents aha + WHERE aha.hook_id = hooks.id AND aha.agent_id = $4 + )) + ) ORDER BY priority DESC, created_at ASC`, string(hookEvent), tenantID, store.MasterTenantID, agentID, ) @@ -426,6 +474,72 @@ func (s *PGHookStore) WriteExecution(ctx context.Context, exec hooks.HookExecuti return nil } +// parseAgentIDsFromAny converts the raw JSON-decoded value (typically +// []interface{} of string UUIDs) into []uuid.UUID. +func parseAgentIDsFromAny(raw any) ([]uuid.UUID, error) { + switch v := raw.(type) { + case []any: + var ids []uuid.UUID + for _, item := range v { + s, ok := item.(string) + if !ok { + continue + } + id, err := uuid.Parse(s) + if err != nil { + return nil, fmt.Errorf("invalid agent_id %q: %w", s, err) + } + ids = append(ids, id) + } + return ids, nil + case []uuid.UUID: + return v, nil + default: + return nil, fmt.Errorf("agent_ids: unsupported type %T", raw) + } +} + +// ─── N:M junction: hook_agents ──────────────────────────────────────── + +func (s *PGHookStore) SetHookAgents(ctx context.Context, hookID uuid.UUID, agentIDs []uuid.UUID) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + if _, err := tx.ExecContext(ctx, "DELETE FROM hook_agents WHERE hook_id = $1", hookID); err != nil { + return fmt.Errorf("clear hook agents: %w", err) + } + for _, aid := range agentIDs { + if _, err := tx.ExecContext(ctx, + "INSERT INTO hook_agents (hook_id, agent_id) VALUES ($1, $2) ON CONFLICT DO NOTHING", + hookID, aid); err != nil { + return fmt.Errorf("insert hook agent: %w", err) + } + } + s.invalidateCache() + return tx.Commit() +} + +func (s *PGHookStore) GetHookAgents(ctx context.Context, hookID uuid.UUID) ([]uuid.UUID, error) { + rows, err := s.db.QueryContext(ctx, + "SELECT agent_id FROM hook_agents WHERE hook_id = $1", hookID) + if err != nil { + return nil, fmt.Errorf("get hook agents: %w", err) + } + defer rows.Close() + var ids []uuid.UUID + for rows.Next() { + var id uuid.UUID + if err := rows.Scan(&id); err != nil { + return nil, err + } + ids = append(ids, id) + } + return ids, rows.Err() +} + // ─── cache helpers ─────────────────────────────────────────────────────────── func (s *PGHookStore) invalidateCache() { @@ -447,7 +561,6 @@ type pgRowScanner interface { func scanHookPGRow(row pgRowScanner) (*hooks.HookConfig, error) { var ( cfg hooks.HookConfig - agentID *uuid.UUID createdBy *uuid.UUID scope, event string handlerType, onTimeout string @@ -456,7 +569,7 @@ func scanHookPGRow(row pgRowScanner) (*hooks.HookConfig, error) { cfgJSON, metaJSON []byte ) err := row.Scan( - &cfg.ID, &cfg.TenantID, &agentID, + &cfg.ID, &cfg.TenantID, &scope, &event, &handlerType, &cfgJSON, &matcher, &ifExpr, &cfg.TimeoutMS, &onTimeout, @@ -473,7 +586,6 @@ func scanHookPGRow(row pgRowScanner) (*hooks.HookConfig, error) { cfg.HandlerType = hooks.HandlerType(handlerType) cfg.OnTimeout = hooks.Decision(onTimeout) cfg.Source = source - cfg.AgentID = agentID cfg.CreatedBy = createdBy if matcher.Valid { diff --git a/internal/store/pg/hooks_test.go b/internal/store/pg/hooks_test.go index e1ac3792..52e6c1cd 100644 --- a/internal/store/pg/hooks_test.go +++ b/internal/store/pg/hooks_test.go @@ -77,8 +77,8 @@ func seedTenantAndAgent(t *testing.T, db *sql.DB) (tenantID, agentID uuid.UUID) t.Fatalf("seed agent: %v", err) } t.Cleanup(func() { - db.Exec("DELETE FROM hook_executions WHERE hook_id IN (SELECT id FROM agent_hooks WHERE tenant_id=$1)", tenantID) - db.Exec("DELETE FROM agent_hooks WHERE tenant_id=$1", tenantID) + db.Exec("DELETE FROM hook_executions WHERE hook_id IN (SELECT id FROM hooks WHERE tenant_id=$1)", tenantID) + db.Exec("DELETE FROM hooks WHERE tenant_id=$1", tenantID) db.Exec("DELETE FROM agents WHERE id=$1", agentID) db.Exec("DELETE FROM tenants WHERE id=$1", tenantID) }) diff --git a/internal/store/sqlitestore/hooks.go b/internal/store/sqlitestore/hooks.go index bd325e1b..c9b0e1e4 100644 --- a/internal/store/sqlitestore/hooks.go +++ b/internal/store/sqlitestore/hooks.go @@ -68,11 +68,7 @@ func (s *SqliteHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uui tid = tenantIDForInsert(ctx) } - var agentID, createdBy, matcher, ifExpr, name *string - if cfg.AgentID != nil { - s := cfg.AgentID.String() - agentID = &s - } + var createdBy, matcher, ifExpr, name *string if cfg.CreatedBy != nil { s := cfg.CreatedBy.String() createdBy = &s @@ -88,13 +84,13 @@ func (s *SqliteHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uui } _, err = s.db.ExecContext(ctx, ` - INSERT INTO agent_hooks - (id, tenant_id, agent_id, scope, event, handler_type, + INSERT INTO hooks + (id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at) - VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,1,?,?,?,?,?,?)`, - id.String(), tid.String(), agentID, + VALUES (?,?,?,?,?,?,?,?,?,?,?,?,1,?,?,?,?,?,?)`, + id.String(), tid.String(), string(cfg.Scope), string(cfg.Event), string(cfg.HandlerType), string(cfgJSON), matcher, ifExpr, cfg.TimeoutMS, string(cfg.OnTimeout), @@ -105,6 +101,17 @@ func (s *SqliteHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uui if err != nil { return uuid.Nil, fmt.Errorf("insert hook: %w", err) } + agentIDs := cfg.AgentIDs + if len(agentIDs) == 0 && cfg.AgentID != nil && *cfg.AgentID != uuid.Nil { + agentIDs = []uuid.UUID{*cfg.AgentID} + } + for _, aid := range agentIDs { + if _, err := s.db.ExecContext(ctx, + `INSERT OR IGNORE INTO hook_agents (hook_id, agent_id) VALUES (?, ?)`, + id.String(), aid.String()); err != nil { + return uuid.Nil, fmt.Errorf("insert hook agent link: %w", err) + } + } s.invalidateCache() return id, nil } @@ -113,11 +120,11 @@ func (s *SqliteHookStore) Create(ctx context.Context, cfg hooks.HookConfig) (uui func (s *SqliteHookStore) GetByID(ctx context.Context, id uuid.UUID) (*hooks.HookConfig, error) { q := ` - SELECT id, tenant_id, agent_id, scope, event, handler_type, + SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at - FROM agent_hooks WHERE id = ?` + FROM hooks WHERE id = ?` args := []any{id.String()} // Tenant-scope guard: non-master callers only see own + global rows. @@ -138,16 +145,19 @@ func (s *SqliteHookStore) GetByID(ctx context.Context, id uuid.UUID) (*hooks.Hoo if err != nil { return nil, fmt.Errorf("get hook by id: %w", err) } + if aids, err := s.GetHookAgents(ctx, cfg.ID); err == nil { + cfg.AgentIDs = aids + } return cfg, nil } // ─── List ──────────────────────────────────────────────────────────────────── func (s *SqliteHookStore) List(ctx context.Context, filter hooks.ListFilter) ([]hooks.HookConfig, error) { - q := `SELECT id, tenant_id, agent_id, scope, event, handler_type, + q := `SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, - created_at, updated_at FROM agent_hooks WHERE 1=1` + created_at, updated_at FROM hooks WHERE 1=1` var args []any if !store.IsMasterScope(ctx) { @@ -163,7 +173,7 @@ func (s *SqliteHookStore) List(ctx context.Context, filter hooks.ListFilter) ([] } if filter.AgentID != nil { - q += " AND agent_id = ?" + q += " AND id IN (SELECT hook_id FROM hook_agents WHERE agent_id = ?)" args = append(args, filter.AgentID.String()) } if filter.Event != nil { @@ -198,7 +208,15 @@ func (s *SqliteHookStore) List(ctx context.Context, filter hooks.ListFilter) ([] } result = append(result, *cfg) } - return result, rows.Err() + if err := rows.Err(); err != nil { + return nil, err + } + for i := range result { + if aids, err := s.GetHookAgents(ctx, result[i].ID); err == nil { + result[i].AgentIDs = aids + } + } + return result, nil } // ─── Update ────────────────────────────────────────────────────────────────── @@ -226,6 +244,18 @@ func (s *SqliteHookStore) Update(ctx context.Context, id uuid.UUID, updates map[ } } + // Handle agent_ids separately — routes to junction table, not a column. + if raw, ok := updates["agent_ids"]; ok { + delete(updates, "agent_ids") + ids, err := parseAgentIDsFromAny(raw) + if err != nil { + return fmt.Errorf("invalid agent_ids: %w", err) + } + if err := s.SetHookAgents(ctx, id, ids); err != nil { + return err + } + } + // Marshal map/slice values to JSON strings for SQLite TEXT columns. for k, v := range updates { switch k { @@ -253,7 +283,7 @@ func (s *SqliteHookStore) Update(ctx context.Context, id uuid.UUID, updates map[ args = append(args, time.Now().UTC().Format(time.RFC3339Nano)) args = append(args, id.String()) - q := fmt.Sprintf("UPDATE agent_hooks SET %s WHERE id = ?", + q := fmt.Sprintf("UPDATE hooks SET %s WHERE id = ?", strings.Join(setClauses, ", ")) if !store.IsMasterScope(ctx) { @@ -291,7 +321,7 @@ func (s *SqliteHookStore) Delete(ctx context.Context, id uuid.UUID) error { } } - q := "DELETE FROM agent_hooks WHERE id = ?" + q := "DELETE FROM hooks WHERE id = ?" args := []any{id.String()} if !store.IsMasterScope(ctx) { @@ -324,10 +354,16 @@ func (s *SqliteHookStore) ResolveForEvent(ctx context.Context, event hooks.Event // Check max version to validate cache freshness. var maxVersion int err := s.db.QueryRowContext(ctx, - `SELECT COALESCE(MAX(version),0) FROM agent_hooks + `SELECT COALESCE(MAX(version),0) FROM hooks WHERE enabled = 1 AND event = ? AND (tenant_id = ? OR tenant_id = ?) - AND (agent_id = ? OR agent_id IS NULL)`, + AND ( + scope IN ('global', 'tenant') + OR (scope = 'agent' AND EXISTS ( + SELECT 1 FROM hook_agents aha + WHERE aha.hook_id = hooks.id AND aha.agent_id = ? + )) + )`, string(hookEvent), tenantID.String(), store.MasterTenantID.String(), agentID.String(), ).Scan(&maxVersion) if err != nil { @@ -344,14 +380,20 @@ func (s *SqliteHookStore) ResolveForEvent(ctx context.Context, event hooks.Event } rows, err := s.db.QueryContext(ctx, ` - SELECT id, tenant_id, agent_id, scope, event, handler_type, + SELECT id, tenant_id, scope, event, handler_type, config, matcher, if_expr, timeout_ms, on_timeout, priority, enabled, version, source, metadata, name, created_by, created_at, updated_at - FROM agent_hooks + FROM hooks WHERE enabled = 1 AND event = ? AND (tenant_id = ? OR tenant_id = ?) - AND (agent_id = ? OR agent_id IS NULL) + AND ( + scope IN ('global', 'tenant') + OR (scope = 'agent' AND EXISTS ( + SELECT 1 FROM hook_agents aha + WHERE aha.hook_id = hooks.id AND aha.agent_id = ? + )) + ) ORDER BY priority DESC, created_at ASC`, string(hookEvent), tenantID.String(), store.MasterTenantID.String(), agentID.String(), ) @@ -431,6 +473,72 @@ func (s *SqliteHookStore) WriteExecution(ctx context.Context, exec hooks.HookExe return nil } +// ─── N:M junction: hook_agents ──────────────────────────────────────── + +func parseAgentIDsFromAny(raw any) ([]uuid.UUID, error) { + switch v := raw.(type) { + case []any: + var ids []uuid.UUID + for _, item := range v { + s, ok := item.(string) + if !ok { + continue + } + id, err := uuid.Parse(s) + if err != nil { + return nil, fmt.Errorf("invalid agent_id %q: %w", s, err) + } + ids = append(ids, id) + } + return ids, nil + case []uuid.UUID: + return v, nil + default: + return nil, fmt.Errorf("agent_ids: unsupported type %T", raw) + } +} + +func (s *SqliteHookStore) SetHookAgents(ctx context.Context, hookID uuid.UUID, agentIDs []uuid.UUID) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + if _, err := tx.ExecContext(ctx, "DELETE FROM hook_agents WHERE hook_id = ?", hookID.String()); err != nil { + return fmt.Errorf("clear hook agents: %w", err) + } + for _, aid := range agentIDs { + if _, err := tx.ExecContext(ctx, + "INSERT OR IGNORE INTO hook_agents (hook_id, agent_id) VALUES (?, ?)", + hookID.String(), aid.String()); err != nil { + return fmt.Errorf("insert hook agent: %w", err) + } + } + s.invalidateCache() + return tx.Commit() +} + +func (s *SqliteHookStore) GetHookAgents(ctx context.Context, hookID uuid.UUID) ([]uuid.UUID, error) { + rows, err := s.db.QueryContext(ctx, + "SELECT agent_id FROM hook_agents WHERE hook_id = ?", hookID.String()) + if err != nil { + return nil, fmt.Errorf("get hook agents: %w", err) + } + defer rows.Close() + var ids []uuid.UUID + for rows.Next() { + var idStr string + if err := rows.Scan(&idStr); err != nil { + return nil, err + } + if id, err := uuid.Parse(idStr); err == nil { + ids = append(ids, id) + } + } + return ids, rows.Err() +} + // ─── cache helpers ─────────────────────────────────────────────────────────── func (s *SqliteHookStore) invalidateCache() { @@ -453,7 +561,6 @@ func scanHookSQLiteRow(row sqliteRowScanner) (*hooks.HookConfig, error) { var ( cfg hooks.HookConfig idStr, tenantStr string - agentIDStr sql.NullString createdByStr sql.NullString scope, event string handlerType, onTimeout string @@ -464,7 +571,7 @@ func scanHookSQLiteRow(row sqliteRowScanner) (*hooks.HookConfig, error) { createdAt, updatedAt sqliteTime ) err := row.Scan( - &idStr, &tenantStr, &agentIDStr, + &idStr, &tenantStr, &scope, &event, &handlerType, &cfgStr, &matcher, &ifExpr, &cfg.TimeoutMS, &onTimeout, @@ -482,11 +589,6 @@ func scanHookSQLiteRow(row sqliteRowScanner) (*hooks.HookConfig, error) { if parsed, err := uuid.Parse(tenantStr); err == nil { cfg.TenantID = parsed } - if agentIDStr.Valid { - if parsed, err := uuid.Parse(agentIDStr.String); err == nil { - cfg.AgentID = &parsed - } - } if createdByStr.Valid { if parsed, err := uuid.Parse(createdByStr.String); err == nil { cfg.CreatedBy = &parsed diff --git a/internal/store/sqlitestore/schema.go b/internal/store/sqlitestore/schema.go index 6244827a..75a20ec5 100644 --- a/internal/store/sqlitestore/schema.go +++ b/internal/store/sqlitestore/schema.go @@ -16,7 +16,7 @@ var schemaSQL string // SchemaVersion is the current SQLite schema version. // Bump this when adding new migration steps below. -const SchemaVersion = 22 +const SchemaVersion = 23 // migrations maps version → SQL to apply when upgrading FROM that version. // schema.sql always represents the LATEST full schema (for fresh DBs). @@ -456,35 +456,28 @@ WHERE context_pruning IS NOT NULL AND json_type(context_pruning) = 'object' AND json_extract(context_pruning, '$.mode') IS NULL;`, - // Version 19 → 20: agent hooks system (mirrors PG migration 000052). - // Creates agent_hooks, hook_executions, tenant_hook_budget tables. - // All idempotent (CREATE TABLE IF NOT EXISTS / CREATE INDEX IF NOT EXISTS). - 19: addAgentHooksTables, + // Version 19 → 20: hooks system (mirrors PG migrations 000052–000055). + // Creates hooks, hook_agents, hook_executions, tenant_hook_budget tables + // with final schema. SQLite/desktop never shipped with intermediate names + // (agent_hooks, agent_hook_agents) so we create the final form directly. + 19: addHooksTables, - // 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). + // Versions 20–22: no-op — consolidated into v19 above. 20: `SELECT 1;`, - - // Version 21 → 22: add name column to agent_hooks. - 21: `ALTER TABLE agent_hooks ADD COLUMN name TEXT;`, + 21: `SELECT 1;`, + 22: `SELECT 1;`, } -// addAgentHooksTables is the SQLite incremental migration for schema v19 → v20. -// Idempotent: CREATE TABLE IF NOT EXISTS / CREATE UNIQUE INDEX IF NOT EXISTS. -// Mirrors PG migration 000052_agent_hooks.up.sql. -// JSONB → TEXT, TIMESTAMPTZ → TEXT (ISO8601), BYTEA → BLOB, DATE → TEXT. -const addAgentHooksTables = ` -CREATE TABLE IF NOT EXISTS agent_hooks ( +// addHooksTables is the SQLite incremental migration for schema v19 → v20. +// Mirrors PG migrations 000052–000055 (consolidated — desktop never shipped +// with intermediate agent_hooks / agent_hook_agents names). +const addHooksTables = ` +CREATE TABLE IF NOT EXISTS 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')), + handler_type TEXT NOT NULL CHECK (handler_type IN ('command', 'http', 'prompt', 'script')), config TEXT NOT NULL DEFAULT '{}', matcher TEXT, if_expr TEXT, @@ -493,27 +486,26 @@ 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 '{}', + name TEXT, 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) + ON hooks (tenant_id, event) WHERE enabled = 1; +CREATE TABLE IF NOT EXISTS hook_agents ( + hook_id TEXT NOT NULL REFERENCES hooks(id) ON DELETE CASCADE, + agent_id TEXT NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + PRIMARY KEY (hook_id, agent_id) +); +CREATE INDEX IF NOT EXISTS idx_hook_agents_agent + ON hook_agents (agent_id); CREATE TABLE IF NOT EXISTS hook_executions ( id TEXT NOT NULL PRIMARY KEY, - hook_id TEXT REFERENCES agent_hooks(id) ON DELETE SET NULL, + hook_id TEXT REFERENCES hooks(id) ON DELETE SET NULL, session_id TEXT, event TEXT NOT NULL, input_hash TEXT, @@ -690,16 +682,6 @@ 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) } } diff --git a/internal/store/sqlitestore/schema.sql b/internal/store/sqlitestore/schema.sql index b6635e2e..1e948755 100644 --- a/internal/store/sqlitestore/schema.sql +++ b/internal/store/sqlitestore/schema.sql @@ -1570,18 +1570,14 @@ CREATE INDEX IF NOT EXISTS idx_vault_links_source WHERE json_extract(metadata, '$.source') IS NOT NULL; -- ============================================================ --- Table: agent_hooks (migrations 000052 + 000053) +-- Table: hooks (renamed from agent_hooks, migration 000055) -- 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 ( +CREATE TABLE IF NOT EXISTS 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')), @@ -1602,16 +1598,29 @@ CREATE TABLE IF NOT EXISTS agent_hooks ( ); CREATE INDEX IF NOT EXISTS idx_hooks_lookup - ON agent_hooks (tenant_id, agent_id, event) + ON hooks (tenant_id, event) WHERE enabled = 1; +-- ============================================================ +-- Table: hook_agents (renamed from agent_hook_agents, N:M junction) +-- ============================================================ + +CREATE TABLE IF NOT EXISTS hook_agents ( + hook_id TEXT NOT NULL REFERENCES hooks(id) ON DELETE CASCADE, + agent_id TEXT NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + PRIMARY KEY (hook_id, agent_id) +); + +CREATE INDEX IF NOT EXISTS idx_hook_agents_agent + ON hook_agents (agent_id); + -- ============================================================ -- Table: hook_executions (append-only audit log, migration 000052) -- ============================================================ CREATE TABLE IF NOT EXISTS hook_executions ( id TEXT NOT NULL PRIMARY KEY, - hook_id TEXT REFERENCES agent_hooks(id) ON DELETE SET NULL, + hook_id TEXT REFERENCES hooks(id) ON DELETE SET NULL, session_id TEXT, event TEXT NOT NULL, input_hash TEXT, diff --git a/internal/store/sqlitestore/schema_rebuild_agent_hooks_v21.go b/internal/store/sqlitestore/schema_rebuild_agent_hooks_v21.go deleted file mode 100644 index 9e192fc0..00000000 --- a/internal/store/sqlitestore/schema_rebuild_agent_hooks_v21.go +++ /dev/null @@ -1,84 +0,0 @@ -//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() -} diff --git a/migrations/000054_agent_hooks_name.down.sql b/migrations/000054_agent_hooks_name.down.sql index c75bef66..9517ffc0 100644 --- a/migrations/000054_agent_hooks_name.down.sql +++ b/migrations/000054_agent_hooks_name.down.sql @@ -1,2 +1,17 @@ --- 000054 down — Remove name column from agent_hooks. +-- 000054 down — Reverse rename, re-add agent_id, drop junction + name. + +ALTER TABLE hooks RENAME TO agent_hooks; +ALTER TABLE hook_agents RENAME TO agent_hook_agents; + +-- Re-add deprecated column (nullable, no data to restore) +ALTER TABLE agent_hooks ADD COLUMN IF NOT EXISTS agent_id UUID REFERENCES agents(id) ON DELETE CASCADE; + +-- Restore original indexes +DROP INDEX IF EXISTS idx_hooks_lookup; +CREATE INDEX idx_hooks_lookup ON agent_hooks (tenant_id, agent_id, event) WHERE enabled = TRUE; +DROP INDEX IF EXISTS idx_hook_agents_agent; +CREATE INDEX idx_hook_agents_agent ON agent_hook_agents (agent_id); + +-- Drop junction table + name column +DROP TABLE IF EXISTS agent_hook_agents; ALTER TABLE agent_hooks DROP COLUMN IF EXISTS name; diff --git a/migrations/000054_agent_hooks_name.up.sql b/migrations/000054_agent_hooks_name.up.sql index 353620b7..f21b507d 100644 --- a/migrations/000054_agent_hooks_name.up.sql +++ b/migrations/000054_agent_hooks_name.up.sql @@ -1,6 +1,41 @@ --- 000054 — Add `name` column to agent_hooks. +-- 000054 — Add `name` column, N:M junction, rename tables, drop agent_id. -- --- User-facing label for hooks. Nullable so existing rows don't break. --- Builtin hooks set name from the YAML spec at seed time. +-- 1) name: user-facing label. Nullable so existing rows don't break. +-- 2) Junction table replaces the 1:N agent_id FK with many-to-many. +-- 3) Data migration: copy existing agent_id values into junction. +-- 4) Rename agent_hooks → hooks, agent_hook_agents → hook_agents. +-- 5) Drop deprecated agent_id column from hooks. +-- 6) Recreate indexes with new table names. +-- Step 1: add name column ALTER TABLE agent_hooks ADD COLUMN IF NOT EXISTS name VARCHAR(255); + +-- Step 2: create junction table +CREATE TABLE IF NOT EXISTS agent_hook_agents ( + hook_id UUID NOT NULL REFERENCES agent_hooks(id) ON DELETE CASCADE, + agent_id UUID NOT NULL REFERENCES agents(id) ON DELETE CASCADE, + PRIMARY KEY (hook_id, agent_id) +); + +CREATE INDEX IF NOT EXISTS idx_hook_agents_agent + ON agent_hook_agents (agent_id); + +-- Step 3: copy existing agent_id values into junction +INSERT INTO agent_hook_agents (hook_id, agent_id) +SELECT id, agent_id FROM agent_hooks +WHERE agent_id IS NOT NULL +ON CONFLICT DO NOTHING; + +-- Step 4: rename tables +ALTER TABLE agent_hooks RENAME TO hooks; +ALTER TABLE agent_hook_agents RENAME TO hook_agents; + +-- Step 5: drop deprecated column +ALTER TABLE hooks DROP COLUMN IF EXISTS agent_id; + +-- Step 6: recreate indexes with new table/column names +DROP INDEX IF EXISTS idx_hooks_lookup; +CREATE INDEX idx_hooks_lookup ON hooks (tenant_id, event) WHERE enabled = TRUE; + +DROP INDEX IF EXISTS idx_hook_agents_agent; +CREATE INDEX idx_hook_agents_agent ON hook_agents (agent_id); diff --git a/tests/integration/hooks_builtin_seed_test.go b/tests/integration/hooks_builtin_seed_test.go index f2325225..90f19c75 100644 --- a/tests/integration/hooks_builtin_seed_test.go +++ b/tests/integration/hooks_builtin_seed_test.go @@ -18,7 +18,7 @@ import ( // Phase 08 — D bucket: builtin seed reconciliation against live PG. // -// Seed() runs against shared state (the PG agent_hooks table) — these tests +// Seed() runs against shared state (the PG hooks table) — these tests // share the test DB with other integration tests, so each one purges the // seeded rows in t.Cleanup. Tests run sequentially via the shared DB lock. @@ -40,7 +40,7 @@ func purgeBuiltinRows(t *testing.T, name string) { for _, ev := range builtinEventList(t, name) { id := builtin.BuiltinEventID(name, ev) db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", id) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", id) + db.Exec("DELETE FROM hooks WHERE id = $1", id) } } @@ -114,7 +114,7 @@ func TestHooksD1b_SeedIdempotent(t *testing.T) { // Cross-check the COUNT — only one row per (id) should exist (PK guard). var n int if err := testDB(t).QueryRow( - `SELECT COUNT(*) FROM agent_hooks WHERE source = 'builtin'`, + `SELECT COUNT(*) FROM hooks WHERE source = 'builtin'`, ).Scan(&n); err != nil { t.Fatalf("count: %v", err) } @@ -143,7 +143,7 @@ func TestHooksD2_VersionBumpUpdatesRows(t *testing.T) { for _, ev := range events { id := builtin.BuiltinEventID(seededBuiltinName, ev) _, err := db.Exec( - `UPDATE agent_hooks SET metadata = jsonb_set(metadata, '{version}', '0'::jsonb) WHERE id = $1`, + `UPDATE hooks SET metadata = jsonb_set(metadata, '{version}', '0'::jsonb) WHERE id = $1`, id, ) if err != nil { @@ -188,7 +188,7 @@ func TestHooksD3_DowngradeSkippedWithWarning(t *testing.T) { for _, ev := range events { id := builtin.BuiltinEventID(seededBuiltinName, ev) if _, err := db.Exec( - `UPDATE agent_hooks SET metadata = jsonb_set(metadata, '{version}', '99'::jsonb) WHERE id = $1`, + `UPDATE hooks SET metadata = jsonb_set(metadata, '{version}', '99'::jsonb) WHERE id = $1`, id, ); err != nil { t.Fatalf("upgrade: %v", err) diff --git a/tests/integration/hooks_chaos_test.go b/tests/integration/hooks_chaos_test.go index b4a9ad0d..16b628c5 100644 --- a/tests/integration/hooks_chaos_test.go +++ b/tests/integration/hooks_chaos_test.go @@ -50,7 +50,7 @@ func TestHooksChaos_HTTPHandler_ProviderDown(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -97,7 +97,7 @@ func TestHooksChaos_PerHookTimeout(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -150,7 +150,7 @@ func TestHooksChaos_CircuitBreaker_AutoDisables(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) // Tight breaker: 3 blocks within 10s. @@ -174,7 +174,7 @@ func TestHooksChaos_CircuitBreaker_AutoDisables(t *testing.T) { // After 3 consecutive blocks, breaker should persist enabled=false. var enabled bool - if err := db.QueryRow(`SELECT enabled FROM agent_hooks WHERE id = $1`, hookID).Scan(&enabled); err != nil { + if err := db.QueryRow(`SELECT enabled FROM hooks WHERE id = $1`, hookID).Scan(&enabled); err != nil { t.Fatalf("read enabled: %v", err) } if enabled { @@ -228,7 +228,7 @@ func TestHooksChaos_RetryDedup_SuppressesDoubleAudit(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) // Write the same dedup_key twice — second INSERT must be suppressed. @@ -289,7 +289,7 @@ func TestHooksChaos_HTTPHandler_5xxRetriesThenErrors(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ diff --git a/tests/integration/hooks_e2e_test.go b/tests/integration/hooks_e2e_test.go index b1ff549d..e6c7eb62 100644 --- a/tests/integration/hooks_e2e_test.go +++ b/tests/integration/hooks_e2e_test.go @@ -70,7 +70,7 @@ func TestHooksE2E_AllEvents_AllowsDefault(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) r, err := d.Fire(ctx, hooks.Event{ @@ -115,7 +115,7 @@ func TestHooksE2E_CommandHandler_BlockOnExitTwo(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -168,7 +168,7 @@ func TestHooksE2E_ContextUpdateInjection(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -216,7 +216,7 @@ func TestHooksE2E_TenantIsolation(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -269,7 +269,7 @@ func TestHooksE2E_EditionGate_CommandOnStandard(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) // Dispatcher with Standard edition command handler — must refuse to run. diff --git a/tests/integration/hooks_phase08_helpers.go b/tests/integration/hooks_phase08_helpers.go index 6f2bd97a..f867e4f4 100644 --- a/tests/integration/hooks_phase08_helpers.go +++ b/tests/integration/hooks_phase08_helpers.go @@ -53,8 +53,8 @@ func seedExtraTenant(t *testing.T, db *sql.DB) uuid.UUID { // the seed-reconciliation tests that re-run Seed() and need a clean slate. func purgeAgentHooksByTenant(t *testing.T, db *sql.DB, tenantID uuid.UUID) { t.Helper() - db.Exec(`DELETE FROM hook_executions WHERE hook_id IN (SELECT id FROM agent_hooks WHERE tenant_id = $1)`, tenantID) - db.Exec(`DELETE FROM agent_hooks WHERE tenant_id = $1`, tenantID) + db.Exec(`DELETE FROM hook_executions WHERE hook_id IN (SELECT id FROM hooks WHERE tenant_id = $1)`, tenantID) + db.Exec(`DELETE FROM hooks WHERE tenant_id = $1`, tenantID) } // purgeAgentHookByID removes one hook row + its audit. Standard cleanup @@ -62,7 +62,7 @@ func purgeAgentHooksByTenant(t *testing.T, db *sql.DB, tenantID uuid.UUID) { func purgeAgentHookByID(t *testing.T, db *sql.DB, id uuid.UUID) { t.Helper() db.Exec(`DELETE FROM hook_executions WHERE hook_id = $1`, id) - db.Exec(`DELETE FROM agent_hooks WHERE id = $1`, id) + db.Exec(`DELETE FROM hooks WHERE id = $1`, id) } // pollAuditDecision polls hook_executions for one row matching (hookID, diff --git a/tests/integration/hooks_pipeline_test.go b/tests/integration/hooks_pipeline_test.go index 6d330ff2..bc706f4b 100644 --- a/tests/integration/hooks_pipeline_test.go +++ b/tests/integration/hooks_pipeline_test.go @@ -75,7 +75,7 @@ func TestHooksIntegration_HTTPHandler_AllowWritesAudit(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -146,7 +146,7 @@ func TestHooksIntegration_HTTPHandler_Block(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ @@ -208,7 +208,7 @@ func TestHooksIntegration_CommandHandler_LiteEdition(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) d := newDispatcher(t, hs, map[hooks.HandlerType]hooks.Handler{ diff --git a/tests/integration/hooks_rbac_test.go b/tests/integration/hooks_rbac_test.go index 1480e050..8607a45a 100644 --- a/tests/integration/hooks_rbac_test.go +++ b/tests/integration/hooks_rbac_test.go @@ -39,7 +39,7 @@ func TestHooksRBAC_StoreTenantIsolation(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookA) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookA) + db.Exec("DELETE FROM hooks WHERE id = $1", hookA) }) hookB, err := hs.Create(tenantCtx(tenantB), hooks.HookConfig{ @@ -56,7 +56,7 @@ func TestHooksRBAC_StoreTenantIsolation(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookB) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookB) + db.Exec("DELETE FROM hooks WHERE id = $1", hookB) }) // List under tenantA ctx → must not include hookB. @@ -130,7 +130,7 @@ func TestHooksRBAC_GlobalScope_VisibleToAllTenants(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", globalHook) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", globalHook) + db.Exec("DELETE FROM hooks WHERE id = $1", globalHook) }) // Both tenants should see the global hook in their list. @@ -176,7 +176,7 @@ func TestHooksRBAC_ResolveForEvent_IncludesGlobalAndTenant(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookT) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookT) + db.Exec("DELETE FROM hooks WHERE id = $1", hookT) }) masterCtx := store.WithCrossTenant(store.WithTenantID(context.Background(), store.MasterTenantID)) @@ -195,7 +195,7 @@ func TestHooksRBAC_ResolveForEvent_IncludesGlobalAndTenant(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookG) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookG) + db.Exec("DELETE FROM hooks WHERE id = $1", hookG) }) got, err := hs.ResolveForEvent(tenantCtx(tenantA), hooks.Event{ diff --git a/tests/integration/hooks_sqlite_v21_rebuild_test.go b/tests/integration/hooks_sqlite_v21_rebuild_test.go deleted file mode 100644 index 2156c11a..00000000 --- a/tests/integration/hooks_sqlite_v21_rebuild_test.go +++ /dev/null @@ -1,141 +0,0 @@ -//go:build sqliteonly - -package integration - -import ( - "database/sql" - "path/filepath" - "testing" - - _ "modernc.org/sqlite" - - "github.com/nextlevelbuilder/goclaw/internal/store/sqlitestore" -) - -// Phase 08 — D6: SQLite v20 → v21 rebuild preserves rows + widens CHECKs. -// -// We pre-construct a DB at schema_version=20 with the OLD agent_hooks CHECK -// constraints (handler_type IN ('command','http','prompt'), source IN -// ('ui','api','seed')) plus 3 hand-inserted rows. Re-opening the DB must: -// - run rebuildAgentHooksV21 outside-of-tx (PRAGMA foreign_keys = OFF) -// - preserve all 3 rows with original IDs -// - bump schema_version to 21 -// - now accept handler_type='script' + source='builtin' inserts -// -// Test isolated under sqliteonly build tag — runs only when the desktop / -// SQLite-only build path is exercised (`go test -tags sqliteonly`). - -const v20AgentHooksDDL = ` -CREATE TABLE agent_hooks ( - id TEXT NOT NULL PRIMARY KEY, - tenant_id TEXT NOT NULL DEFAULT '0193a5b0-7000-7000-8000-000000000001', - agent_id TEXT, - 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')), - 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')), - 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 INDEX IF NOT EXISTS idx_hooks_lookup - ON agent_hooks (tenant_id, agent_id, event) - WHERE enabled = 1; -` - -func TestHooksD6_SQLiteV21RebuildPreservesRows(t *testing.T) { - dbPath := filepath.Join(t.TempDir(), "hooks-rebuild.db") - - // ─── Stage 1: pre-seed v20 state ───────────────────────────────────────── - preIDs := []string{ - "11111111-1111-1111-1111-111111111111", - "22222222-2222-2222-2222-222222222222", - "33333333-3333-3333-3333-333333333333", - } - - { - db, err := sql.Open("sqlite", dbPath) - if err != nil { - t.Fatalf("open v20 db: %v", err) - } - // schema_version table + entry at 20. - if _, err := db.Exec(`CREATE TABLE schema_version (version INTEGER NOT NULL PRIMARY KEY)`); err != nil { - t.Fatalf("create schema_version: %v", err) - } - if _, err := db.Exec(`INSERT INTO schema_version VALUES (20)`); err != nil { - t.Fatalf("seed schema_version: %v", err) - } - // Minimal parent tables needed: seedMasterTenant() requires - // (id, name, slug, status) on tenants; rebuilt agent_hooks FKs agents(id). - if _, err := db.Exec(`CREATE TABLE tenants (id TEXT PRIMARY KEY, name TEXT, slug TEXT, status TEXT)`); err != nil { - t.Fatalf("create tenants: %v", err) - } - if _, err := db.Exec(`CREATE TABLE agents (id TEXT PRIMARY KEY)`); err != nil { - t.Fatalf("create agents: %v", err) - } - // Inline v20 agent_hooks DDL. - if _, err := db.Exec(v20AgentHooksDDL); err != nil { - t.Fatalf("create v20 agent_hooks: %v", err) - } - // 3 rows with handler_type='command' (allowed under v20 CHECK). - for _, id := range preIDs { - if _, err := db.Exec( - `INSERT INTO agent_hooks (id, scope, event, handler_type) VALUES (?, 'tenant', 'pre_tool_use', 'command')`, - id, - ); err != nil { - t.Fatalf("insert v20 row %s: %v", id, err) - } - } - if err := db.Close(); err != nil { - t.Fatalf("close v20 db: %v", err) - } - } - - // ─── Stage 2: re-open via EnsureSchema → triggers rebuild ─────────────── - db, err := sqlitestore.OpenDB(dbPath) - if err != nil { - t.Fatalf("re-open: %v", err) - } - defer db.Close() - if err := sqlitestore.EnsureSchema(db); err != nil { - t.Fatalf("EnsureSchema (rebuild): %v", err) - } - - // schema_version must now be 21. - var ver int - if err := db.QueryRow(`SELECT version FROM schema_version LIMIT 1`).Scan(&ver); err != nil { - t.Fatalf("read schema_version: %v", err) - } - if ver != sqlitestore.SchemaVersion { - t.Fatalf("schema_version=%d, want %d", ver, sqlitestore.SchemaVersion) - } - - // All 3 pre-seeded rows must still be present with original IDs. - for _, id := range preIDs { - var got string - err := db.QueryRow(`SELECT id FROM agent_hooks WHERE id = ?`, id).Scan(&got) - if err != nil { - t.Errorf("row %s missing after rebuild: %v", id, err) - } - if got != id { - t.Errorf("row id mismatch: got %s want %s", got, id) - } - } - - // New CHECK widened: handler_type='script' + source='builtin' must insert. - if _, err := db.Exec( - `INSERT INTO agent_hooks (id, scope, event, handler_type, source) VALUES (?, 'global', 'user_prompt_submit', 'script', 'builtin')`, - "deadbeef-aaaa-bbbb-cccc-000000000001", - ); err != nil { - t.Fatalf("post-rebuild script+builtin insert: %v", err) - } -} diff --git a/tests/integration/hooks_tracing_test.go b/tests/integration/hooks_tracing_test.go index c6b4599e..8afde48d 100644 --- a/tests/integration/hooks_tracing_test.go +++ b/tests/integration/hooks_tracing_test.go @@ -114,7 +114,7 @@ func TestHooksTracing_DispatcherEmitsSpan(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) // Wrap ctx with tracing collector + trace ID — dispatcher's emit helper diff --git a/tests/integration/v3_hooks_store_test.go b/tests/integration/v3_hooks_store_test.go index 9749de8c..707eebe9 100644 --- a/tests/integration/v3_hooks_store_test.go +++ b/tests/integration/v3_hooks_store_test.go @@ -50,7 +50,7 @@ func TestHooksStore_CRUDRoundTrip(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", id) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", id) + db.Exec("DELETE FROM hooks WHERE id = $1", id) }) got, err := s.GetByID(ctx, id) @@ -120,7 +120,7 @@ func TestHooksStore_TenantIsolation(t *testing.T) { t.Fatalf("Create tenantA hook: %v", err) } t.Cleanup(func() { - db.Exec("DELETE FROM agent_hooks WHERE id = $1", id) + db.Exec("DELETE FROM hooks WHERE id = $1", id) }) // Tenant B must not see A's row. @@ -170,7 +170,7 @@ func TestHooksStore_DedupKeyIdempotent(t *testing.T) { } t.Cleanup(func() { db.Exec("DELETE FROM hook_executions WHERE hook_id = $1", hookID) - db.Exec("DELETE FROM agent_hooks WHERE id = $1", hookID) + db.Exec("DELETE FROM hooks WHERE id = $1", hookID) }) exec := hooks.HookExecution{ diff --git a/ui/web/src/api/protocol.ts b/ui/web/src/api/protocol.ts index 88582796..da541fe9 100644 --- a/ui/web/src/api/protocol.ts +++ b/ui/web/src/api/protocol.ts @@ -70,6 +70,7 @@ export const Methods = { CONFIG_APPLY: "config.apply", CONFIG_PATCH: "config.patch", CONFIG_SCHEMA: "config.schema", + CONFIG_DEFAULTS: "config.defaults", // Sessions SESSIONS_LIST: "sessions.list", diff --git a/ui/web/src/hooks/use-hooks.ts b/ui/web/src/hooks/use-hooks.ts index dfe1c091..075ceaba 100644 --- a/ui/web/src/hooks/use-hooks.ts +++ b/ui/web/src/hooks/use-hooks.ts @@ -9,6 +9,7 @@ export interface HookConfig { id: string; tenant_id: string; agent_id?: string | null; + agent_ids?: string[]; name?: string; event: string; handler_type: "command" | "http" | "prompt" | "script"; diff --git a/ui/web/src/i18n/locales/en/agents.json b/ui/web/src/i18n/locales/en/agents.json index 01662476..c109ad94 100644 --- a/ui/web/src/i18n/locales/en/agents.json +++ b/ui/web/src/i18n/locales/en/agents.json @@ -593,7 +593,7 @@ "saving": "Saving...", "saved": "Saved", "failedToSave": "Failed to save", - "usingGlobalDefaults": "Using global defaults from config.json" + "usingGlobalDefaults": "Using system defaults" }, "files": { "readOnly": "Read-only", @@ -743,6 +743,7 @@ "maxSpawnDepth": "Max Spawn Depth", "maxChildrenPerAgent": "Max Children Per Agent", "archiveAfter": "Archive After (minutes)", + "maxRetries": "Max Retries", "modelOverride": "Model Override", "inheritFromAgent": "(inherit from agent)" }, diff --git a/ui/web/src/i18n/locales/en/hooks.json b/ui/web/src/i18n/locales/en/hooks.json index 47870c6b..097f1ff2 100644 --- a/ui/web/src/i18n/locales/en/hooks.json +++ b/ui/web/src/i18n/locales/en/hooks.json @@ -54,6 +54,8 @@ "title_edit": "Edit Hook", "name": "Name", "namePlaceholder": "e.g. Block sensitive tools", + "agents": "Agents", + "agentsHint": "Select which agents this hook applies to", "event": "Event", "handlerType": "Handler Type", "scope": "Scope", diff --git a/ui/web/src/i18n/locales/vi/agents.json b/ui/web/src/i18n/locales/vi/agents.json index 41b455e2..cd8b25a9 100644 --- a/ui/web/src/i18n/locales/vi/agents.json +++ b/ui/web/src/i18n/locales/vi/agents.json @@ -577,7 +577,7 @@ "saving": "Đang lưu...", "saved": "Đã lưu", "failedToSave": "Lưu thất bại", - "usingGlobalDefaults": "Đang dùng mặc định toàn cục từ config.json" + "usingGlobalDefaults": "Đang dùng mặc định hệ thống" }, "files": { "readOnly": "Chỉ đọc", @@ -727,6 +727,7 @@ "maxSpawnDepth": "Độ sâu sinh sản tối đa", "maxChildrenPerAgent": "Con tối đa mỗi agent", "archiveAfter": "Lưu trữ sau (phút)", + "maxRetries": "Số lần thử lại tối đa", "modelOverride": "Ghi đè model", "inheritFromAgent": "(kế thừa từ agent)" }, diff --git a/ui/web/src/i18n/locales/vi/hooks.json b/ui/web/src/i18n/locales/vi/hooks.json index 8433dca3..32d6cfef 100644 --- a/ui/web/src/i18n/locales/vi/hooks.json +++ b/ui/web/src/i18n/locales/vi/hooks.json @@ -54,6 +54,8 @@ "title_edit": "Sửa Hook", "name": "Tên", "namePlaceholder": "VD: Chặn tool nhạy cảm", + "agents": "Agents", + "agentsHint": "Chọn agents mà hook này áp dụng", "event": "Sự kiện", "handlerType": "Loại xử lý", "scope": "Phạm vi", diff --git a/ui/web/src/i18n/locales/zh/agents.json b/ui/web/src/i18n/locales/zh/agents.json index 63765612..cc260577 100644 --- a/ui/web/src/i18n/locales/zh/agents.json +++ b/ui/web/src/i18n/locales/zh/agents.json @@ -577,7 +577,7 @@ "saving": "保存中...", "saved": "已保存", "failedToSave": "保存失败", - "usingGlobalDefaults": "使用 config.json 中的全局默认值" + "usingGlobalDefaults": "使用系统默认值" }, "files": { "readOnly": "只读", @@ -727,6 +727,7 @@ "maxSpawnDepth": "最大生成深度", "maxChildrenPerAgent": "每Agent最大子数", "archiveAfter": "归档时间（分钟）", + "maxRetries": "最大重试次数", "modelOverride": "模型覆盖", "inheritFromAgent": "（继承自Agent）" }, diff --git a/ui/web/src/i18n/locales/zh/hooks.json b/ui/web/src/i18n/locales/zh/hooks.json index cbfaacea..e4f55b61 100644 --- a/ui/web/src/i18n/locales/zh/hooks.json +++ b/ui/web/src/i18n/locales/zh/hooks.json @@ -54,6 +54,8 @@ "title_edit": "编辑钩子", "name": "名称", "namePlaceholder": "例如：阻止敏感工具", + "agents": "Agents", + "agentsHint": "选择此钩子适用的 Agent", "event": "事件", "handlerType": "处理器类型", "scope": "范围", diff --git a/ui/web/src/lib/query-keys.ts b/ui/web/src/lib/query-keys.ts index 3b3835f8..51834646 100644 --- a/ui/web/src/lib/query-keys.ts +++ b/ui/web/src/lib/query-keys.ts @@ -61,6 +61,7 @@ export const queryKeys = { }, config: { all: ["config"] as const, + defaults: ["config", "defaults"] as const, }, tts: { all: ["tts"] as const, diff --git a/ui/web/src/pages/agents/agent-detail/config-sections/context-pruning-section.tsx b/ui/web/src/pages/agents/agent-detail/config-sections/context-pruning-section.tsx index 5476fb03..1309d4d4 100644 --- a/ui/web/src/pages/agents/agent-detail/config-sections/context-pruning-section.tsx +++ b/ui/web/src/pages/agents/agent-detail/config-sections/context-pruning-section.tsx @@ -4,6 +4,7 @@ import { ChevronRight } from "lucide-react"; import { Input } from "@/components/ui/input"; import { Switch } from "@/components/ui/switch"; import type { ContextPruningConfig } from "@/types/agent"; +import { useConfigDefaults } from "@/pages/config/hooks/use-config-defaults"; import { ConfigSection, InfoLabel, numOrUndef } from "./config-section"; interface ContextPruningSectionProps { @@ -17,6 +18,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co const { t } = useTranslation("agents"); const s = "configSections.contextPruning"; const [showAdvanced, setShowAdvanced] = useState(false); + const d = useConfigDefaults().agents.contextPruning; return ( {t(`${s}.keepLastAssistants`)} onChange({ ...value, keepLastAssistants: numOrUndef(e.target.value) }) @@ -44,7 +46,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co {t(`${s}.cacheTtl`, "Cache TTL")} onChange({ ...value, ttl: e.target.value || undefined })} /> @@ -70,7 +72,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co onChange({ ...value, softTrimRatio: numOrUndef(e.target.value) })} /> @@ -80,7 +82,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co onChange({ ...value, hardClearRatio: numOrUndef(e.target.value) })} /> @@ -93,7 +95,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co {t(`${s}.maxChars`)} onChange({ ...value, softTrim: { ...value.softTrim, maxChars: numOrUndef(e.target.value) } }) @@ -104,7 +106,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co {t(`${s}.headChars`)} onChange({ ...value, softTrim: { ...value.softTrim, headChars: numOrUndef(e.target.value) } }) @@ -115,7 +117,7 @@ export function ContextPruningSection({ enabled, value, onToggle, onChange }: Co {t(`${s}.tailChars`)} onChange({ ...value, softTrim: { ...value.softTrim, tailChars: numOrUndef(e.target.value) } }) diff --git a/ui/web/src/pages/agents/agent-detail/config-sections/subagents-section.tsx b/ui/web/src/pages/agents/agent-detail/config-sections/subagents-section.tsx index 63598104..157e101c 100644 --- a/ui/web/src/pages/agents/agent-detail/config-sections/subagents-section.tsx +++ b/ui/web/src/pages/agents/agent-detail/config-sections/subagents-section.tsx @@ -13,6 +13,7 @@ import type { SubagentsConfig } from "@/types/agent"; import { ConfigSection, InfoLabel, numOrUndef } from "./config-section"; import { useProviders } from "@/pages/providers/hooks/use-providers"; import { useProviderModels } from "@/pages/providers/hooks/use-provider-models"; +import { useConfigDefaults } from "@/pages/config/hooks/use-config-defaults"; interface SubagentsSectionProps { enabled: boolean; @@ -28,6 +29,7 @@ export function SubagentsSection({ enabled, value, onToggle, onChange }: Subagen const enabledProviders = providers.filter((p) => p.enabled); const defaultProvider = useMemo(() => enabledProviders[0], [enabledProviders]); const { models, loading: modelsLoading } = useProviderModels(defaultProvider?.id); + const d = useConfigDefaults().agents.subagents; return ( {t(`${s}.maxConcurrent`)} onChange({ ...value, maxConcurrent: numOrUndef(e.target.value) })} /> @@ -52,7 +54,7 @@ export function SubagentsSection({ enabled, value, onToggle, onChange }: Subagen value={String(value.maxSpawnDepth ?? "")} onValueChange={(v) => onChange({ ...value, maxSpawnDepth: Number(v) })} > - + {[1, 2, 3, 4, 5].map((n) => ( {n} @@ -66,7 +68,7 @@ export function SubagentsSection({ enabled, value, onToggle, onChange }: Subagen {t(`${s}.maxChildrenPerAgent`)} onChange({ ...value, maxChildrenPerAgent: numOrUndef(e.target.value) })} /> @@ -75,12 +77,25 @@ export function SubagentsSection({ enabled, value, onToggle, onChange }: Subagen {t(`${s}.archiveAfter`)} onChange({ ...value, archiveAfterMinutes: numOrUndef(e.target.value) })} /> +
+
+ {t(`${s}.maxRetries`, "Max retries")} + onChange({ ...value, maxRetries: numOrUndef(e.target.value) })} + /> +
+
{t(`${s}.modelOverride`)} s.connected); + + const { data } = useQuery({ + queryKey: queryKeys.config.defaults, + queryFn: () => ws.call(Methods.CONFIG_DEFAULTS), + staleTime: 60_000, + enabled: connected, + initialData: CONFIG_DEFAULTS_FALLBACK, + }); + + return data; +} diff --git a/ui/web/src/pages/hooks/components/hook-form-dialog.tsx b/ui/web/src/pages/hooks/components/hook-form-dialog.tsx index 06caf557..8fd8f369 100644 --- a/ui/web/src/pages/hooks/components/hook-form-dialog.tsx +++ b/ui/web/src/pages/hooks/components/hook-form-dialog.tsx @@ -15,6 +15,7 @@ import { RadioGroup, RadioGroupItem } from "@/components/ui/radio-group"; import { hookFormSchema, type HookFormData } from "@/schemas/hooks.schema"; import type { HookConfig } from "@/hooks/use-hooks"; import { useAuthStore } from "@/stores/use-auth-store"; +import { useAgents } from "@/pages/agents/hooks/use-agents"; import { ScriptEditor } from "./script-editor"; const HOOK_EVENTS = [ @@ -44,6 +45,7 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo resolver: zodResolver(hookFormSchema), defaultValues: { name: "", + agent_ids: [], event: "pre_tool_use", handler_type: "script", scope: "tenant", @@ -57,6 +59,8 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo }); const handlerType = watch("handler_type"); + const scope = watch("scope"); + const { agents } = useAgents(); // Builtin rows (Phase 04/05) ship with source='builtin'. UI + backend agree: // only `enabled` is mutable. All other inputs render as read-only, and the // Save button label changes to reflect the narrowed scope. @@ -75,6 +79,7 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo initial.handler_type === "command" ? "http" : initial.handler_type; reset({ name: initial.name ?? "", + agent_ids: initial.agent_ids ?? [], event: initial.event as HookFormData["event"], handler_type: handlerType, scope: initial.scope, @@ -164,6 +169,36 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo
+ {/* Agent picker — visible only when scope=agent */} + {scope === "agent" && agents.length > 0 && ( + ( +
+ +
+ {agents.map((a) => { + const checked = (field.value ?? []).includes(a.id); + return ( + + ); + })} +
+

{t("form.agentsHint")}

+
+ )} /> + )} + {/* Handler type — `command` intentionally absent post-Wave-1. Lite users keep existing rows but cannot create new ones via UI (DB/CLI path only). `script` added in Phase 06 (goja sandbox; builtin PII redactor = Phase 05). */} diff --git a/ui/web/src/pages/hooks/components/hook-list-row.tsx b/ui/web/src/pages/hooks/components/hook-list-row.tsx index bb24f7d8..39015155 100644 --- a/ui/web/src/pages/hooks/components/hook-list-row.tsx +++ b/ui/web/src/pages/hooks/components/hook-list-row.tsx @@ -62,6 +62,11 @@ export function HookListRow({ hook, onClick, onToggle, onEdit, onDelete, onTest {hook.scope} {hook.source === "builtin" && } + {hook.scope === "agent" && hook.agent_ids && hook.agent_ids.length > 0 && ( + + {hook.agent_ids.length} agent{hook.agent_ids.length > 1 ? "s" : ""} + + )} {hook.matcher && (

{hook.matcher}

diff --git a/ui/web/src/pages/hooks/hooks-page.tsx b/ui/web/src/pages/hooks/hooks-page.tsx index f78d2741..abef2dc3 100644 --- a/ui/web/src/pages/hooks/hooks-page.tsx +++ b/ui/web/src/pages/hooks/hooks-page.tsx @@ -121,6 +121,7 @@ export function HooksPage() { } await createMutation.mutateAsync({ name: data.name ?? "", + agent_ids: data.agent_ids ?? [], event: data.event, handler_type: data.handler_type, scope: data.scope, @@ -147,6 +148,7 @@ export function HooksPage() { hookId: editTarget.id, updates: { name: data.name ?? "", + agent_ids: data.agent_ids ?? [], event: data.event, handler_type: data.handler_type, scope: data.scope, diff --git a/ui/web/src/schemas/hooks.schema.ts b/ui/web/src/schemas/hooks.schema.ts index 38d2ba16..0678b4f5 100644 --- a/ui/web/src/schemas/hooks.schema.ts +++ b/ui/web/src/schemas/hooks.schema.ts @@ -21,6 +21,7 @@ export const HookScopeEnum = z.enum(["global", "tenant", "agent"]); export const hookFormSchema = z .object({ name: z.string().max(255).optional(), + agent_ids: z.array(z.string()).optional(), event: HookEventEnum, handler_type: HookHandlerTypeEnum, scope: HookScopeEnum, diff --git a/ui/web/src/types/agent.ts b/ui/web/src/types/agent.ts index 6a585c9e..4b65296a 100644 --- a/ui/web/src/types/agent.ts +++ b/ui/web/src/types/agent.ts @@ -16,6 +16,7 @@ export interface SubagentsConfig { maxSpawnDepth?: number; maxChildrenPerAgent?: number; archiveAfterMinutes?: number; + maxRetries?: number; model?: string; }