From fe5a3c7f921ce5b5f0151fda1787363877044847 Mon Sep 17 00:00:00 2001 From: ntduc Date: Wed, 22 Jul 2026 19:49:15 +0700 Subject: [PATCH] fix(hooks): select prompt provider and model --- docs/agent-hooks.md | 4 +- internal/hooks/handlers/prompt.go | 25 +++++-- internal/hooks/handlers/prompt_resolver.go | 46 ++++++++---- .../hooks/handlers/prompt_resolver_test.go | 72 +++++++++++++++++++ internal/hooks/handlers/prompt_test.go | 50 +++++++++++-- ui/web/src/i18n/locales/en/hooks.json | 5 +- ui/web/src/i18n/locales/ru/hooks.json | 3 + ui/web/src/i18n/locales/vi/hooks.json | 5 +- ui/web/src/i18n/locales/zh/hooks.json | 5 +- .../agents/agent-detail/agent-hooks-tab.tsx | 41 +---------- .../pages/hooks/__tests__/hooks-page.test.ts | 53 ++++++++++++-- .../hooks/components/hook-form-dialog.tsx | 44 ++++++++---- .../hooks/components/hook-overview-tab.tsx | 14 +++- ui/web/src/pages/hooks/hook-form-config.ts | 39 ++++++++++ ui/web/src/pages/hooks/hooks-page.tsx | 46 +----------- ui/web/src/schemas/hooks.schema.ts | 15 ++++ 16 files changed, 335 insertions(+), 132 deletions(-) create mode 100644 internal/hooks/handlers/prompt_resolver_test.go create mode 100644 ui/web/src/pages/hooks/hook-form-config.ts diff --git a/docs/agent-hooks.md b/docs/agent-hooks.md index 1612500a..69897868 100644 --- a/docs/agent-hooks.md +++ b/docs/agent-hooks.md @@ -107,7 +107,8 @@ function handle(event) { "matcher": "^(exec|shell|write_file)$", "config": { "prompt_template": "Evaluate safety of this tool call.", - "model": "haiku", + "provider": "openai", + "model": "gpt-4.1-mini", "max_invocations_per_turn": 5 } } @@ -116,6 +117,7 @@ function handle(event) { Required: - `prompt_template` — system-level instruction the evaluator receives. - `matcher` or `if_expr` — runaway-cost guard; prevents firing the LLM on every event. +- `provider` and `model` — required for prompt hooks created or edited in the dashboard; select the pair from the configured provider catalog. Legacy model-only hooks remain supported at runtime. Safeguards: - **Structured output**: evaluator MUST call a `decide(decision, reason, injection_detected, updated_input)` tool. Free-text responses fail-closed. diff --git a/internal/hooks/handlers/prompt.go b/internal/hooks/handlers/prompt.go index 062c3c1f..f4daad59 100644 --- a/internal/hooks/handlers/prompt.go +++ b/internal/hooks/handlers/prompt.go @@ -24,11 +24,11 @@ import ( // ── Public surface ────────────────────────────────────────────────────────── // ProviderResolver returns a provider + resolved model name for a given -// (tenantID, preferredModel). preferredModel is the UI/config-specified -// model (e.g. "haiku"); resolver may expand aliases or fall back to the -// tenant's default when the alias is unknown. +// tenant. preferredProvider and preferredModel come from hook config. Legacy +// hooks may omit preferredProvider, in which case the resolver applies its +// backward-compatible alias/config fallback chain. type ProviderResolver interface { - ResolveForHook(ctx context.Context, tenantID uuid.UUID, preferredModel string) (providers.Provider, string, error) + ResolveForHook(ctx context.Context, tenantID uuid.UUID, preferredProvider, preferredModel string) (providers.Provider, string, error) } // PromptHandler implements hooks.Handler via an LLM structured-output call. @@ -150,8 +150,9 @@ func (h *PromptHandler) Execute(ctx context.Context, cfg hooks.HookConfig, ev ho } // 4. Resolve provider - model := h.modelFor(cfg) - provider, resolvedModel, err := h.Resolver.ResolveForHook(ctx, ev.TenantID, model) + providerName := h.providerFor(cfg) + model := h.modelFor(cfg, providerName != "") + provider, resolvedModel, err := h.Resolver.ResolveForHook(ctx, ev.TenantID, providerName, model) if err != nil || provider == nil { return hooks.DecisionError, fmt.Errorf("hook: prompt handler: resolve provider: %w", err) } @@ -233,10 +234,20 @@ func (h *PromptHandler) maxInvocations(cfg hooks.HookConfig) int { return defaultPromptMaxInvocations } -func (h *PromptHandler) modelFor(cfg hooks.HookConfig) string { +func (h *PromptHandler) providerFor(cfg hooks.HookConfig) string { + provider, _ := cfg.Config["provider"].(string) + return strings.TrimSpace(provider) +} + +func (h *PromptHandler) modelFor(cfg hooks.HookConfig, hasExplicitProvider bool) string { if m, _ := cfg.Config["model"].(string); m != "" { return m } + // An explicit provider with no model means "use that provider's default". + // Only legacy model-only hooks inherit the historical Haiku fallback. + if hasExplicitProvider { + return "" + } if h.DefaultModel != "" { return h.DefaultModel } diff --git a/internal/hooks/handlers/prompt_resolver.go b/internal/hooks/handlers/prompt_resolver.go index a27c1ca7..b817b66d 100644 --- a/internal/hooks/handlers/prompt_resolver.go +++ b/internal/hooks/handlers/prompt_resolver.go @@ -3,6 +3,7 @@ package handlers import ( "context" "errors" + "fmt" "strings" "github.com/google/uuid" @@ -15,17 +16,18 @@ import ( // the ProviderResolver interface consumed by PromptHandler. It applies a // simple fallback chain: // -// 1. Explicit model alias → map to provider by convention (haiku/sonnet/opus → anthropic). -// 2. System config `hooks.prompt.provider` / `hooks.prompt.model`. -// 3. System config `background.provider` / `background.model`. -// 4. First registered provider for the tenant. +// 1. Explicit provider + model from the hook config. +// 2. Legacy explicit model alias → map to provider by convention. +// 3. System config `hooks.prompt.provider` / `hooks.prompt.model`. +// 4. System config `background.provider` / `background.model`. +// 5. First registered provider for the tenant. // // Keeping this adapter in `handlers` rather than the higher-level // `providerresolve` package avoids a new import cycle (providerresolve → // store → hooks would introduce a diamond). type RegistryResolver struct { - Registry *providers.Registry - SysConfig store.SystemConfigStore + Registry *providers.Registry + SysConfig store.SystemConfigStore DefaultProviderForAlias func(alias string) string } @@ -33,19 +35,36 @@ type RegistryResolver struct { // registry MUST be non-nil. sysConfig may be nil (fallback to step 4 only). func NewRegistryResolver(registry *providers.Registry, sysConfig store.SystemConfigStore) *RegistryResolver { return &RegistryResolver{ - Registry: registry, - SysConfig: sysConfig, + Registry: registry, + SysConfig: sysConfig, DefaultProviderForAlias: defaultProviderForAlias, } } // ResolveForHook implements ProviderResolver. -func (r *RegistryResolver) ResolveForHook(ctx context.Context, tenantID uuid.UUID, preferredModel string) (providers.Provider, string, error) { +func (r *RegistryResolver) ResolveForHook(ctx context.Context, tenantID uuid.UUID, preferredProvider, preferredModel string) (providers.Provider, string, error) { if r == nil || r.Registry == nil { return nil, "", errors.New("hook resolver: nil registry") } - // Step 1: explicit alias → provider name. + // Step 1: an explicit provider is authoritative. Never silently route its + // model to another provider when a hook is misconfigured. + if providerName := strings.TrimSpace(preferredProvider); providerName != "" { + p, err := r.Registry.GetForTenant(tenantID, providerName) + if err != nil { + return nil, "", fmt.Errorf("hook resolver: configured provider %q: %w", providerName, err) + } + if p == nil { + return nil, "", fmt.Errorf("hook resolver: configured provider %q returned nil", providerName) + } + model := strings.TrimSpace(preferredModel) + if model == "" { + model = p.DefaultModel() + } + return p, model, nil + } + + // Step 2: legacy explicit alias → provider name. if preferredModel != "" { if name := r.providerForAlias(preferredModel); name != "" { if p, err := r.Registry.GetForTenant(tenantID, name); err == nil && p != nil { @@ -54,18 +73,19 @@ func (r *RegistryResolver) ResolveForHook(ctx context.Context, tenantID uuid.UUI } } - // Step 2: system config hooks.prompt.* + // Step 3: system config hooks.prompt.* configs := r.loadConfigs(ctx, tenantID) if p, m, ok := r.tryConfig(tenantID, configs["hooks.prompt.provider"], configs["hooks.prompt.model"], preferredModel); ok { return p, m, nil } - // Step 3: fall back to background.* + // Step 4: fall back to background.* if p, m, ok := r.tryConfig(tenantID, configs["background.provider"], configs["background.model"], preferredModel); ok { return p, m, nil } - // Step 4: first registered provider for the tenant. + // Step 5: first registered provider for the tenant. This path remains only + // for backward compatibility with hooks that predate explicit providers. names := r.Registry.ListForTenant(tenantID) if len(names) == 0 { return nil, "", errors.New("hook resolver: no providers registered for tenant") diff --git a/internal/hooks/handlers/prompt_resolver_test.go b/internal/hooks/handlers/prompt_resolver_test.go new file mode 100644 index 00000000..76f09a31 --- /dev/null +++ b/internal/hooks/handlers/prompt_resolver_test.go @@ -0,0 +1,72 @@ +package handlers_test + +import ( + "context" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/hooks/handlers" + "github.com/nextlevelbuilder/goclaw/internal/providers" +) + +func TestRegistryResolver_ExplicitProviderIsAuthoritative(t *testing.T) { + tenantID := uuid.New() + registry := providers.NewRegistry(nil) + registry.RegisterForTenant(tenantID, &fakeProvider{name: "cppai", defaultModel: "gpt-default"}) + registry.RegisterForTenant(tenantID, &fakeProvider{name: "anthropic", defaultModel: "claude-default"}) + resolver := handlers.NewRegistryResolver(registry, nil) + + provider, model, err := resolver.ResolveForHook(context.Background(), tenantID, "cppai", "gpt-5.6-terra") + if err != nil { + t.Fatalf("ResolveForHook() error: %v", err) + } + if provider.Name() != "cppai" || model != "gpt-5.6-terra" { + t.Fatalf("resolved=(%q, %q), want (cppai, gpt-5.6-terra)", provider.Name(), model) + } +} + +func TestRegistryResolver_ExplicitProviderUsesItsDefaultModel(t *testing.T) { + tenantID := uuid.New() + registry := providers.NewRegistry(nil) + registry.RegisterForTenant(tenantID, &fakeProvider{name: "bailian", defaultModel: "qwen3.7-plus"}) + resolver := handlers.NewRegistryResolver(registry, nil) + + provider, model, err := resolver.ResolveForHook(context.Background(), tenantID, "bailian", "") + if err != nil { + t.Fatalf("ResolveForHook() error: %v", err) + } + if provider.Name() != "bailian" || model != "qwen3.7-plus" { + t.Fatalf("resolved=(%q, %q), want (bailian, qwen3.7-plus)", provider.Name(), model) + } +} + +func TestRegistryResolver_MissingExplicitProviderDoesNotFallback(t *testing.T) { + tenantID := uuid.New() + registry := providers.NewRegistry(nil) + registry.RegisterForTenant(tenantID, &fakeProvider{name: "bailian", defaultModel: "qwen3.7-plus"}) + resolver := handlers.NewRegistryResolver(registry, nil) + + provider, model, err := resolver.ResolveForHook(context.Background(), tenantID, "missing", "gpt-5.6-terra") + if err == nil { + t.Fatal("ResolveForHook() error=nil, want configured-provider error") + } + if provider != nil || model != "" { + t.Fatalf("resolved=(%v, %q), want nil provider and empty model", provider, model) + } +} + +func TestRegistryResolver_LegacyModelAliasStillResolves(t *testing.T) { + tenantID := uuid.New() + registry := providers.NewRegistry(nil) + registry.RegisterForTenant(tenantID, &fakeProvider{name: "anthropic", defaultModel: "claude-default"}) + resolver := handlers.NewRegistryResolver(registry, nil) + + provider, model, err := resolver.ResolveForHook(context.Background(), tenantID, "", "haiku") + if err != nil { + t.Fatalf("ResolveForHook() error: %v", err) + } + if provider.Name() != "anthropic" || model != "haiku" { + t.Fatalf("resolved=(%q, %q), want (anthropic, haiku)", provider.Name(), model) + } +} diff --git a/internal/hooks/handlers/prompt_test.go b/internal/hooks/handlers/prompt_test.go index d3d02e5a..22112798 100644 --- a/internal/hooks/handlers/prompt_test.go +++ b/internal/hooks/handlers/prompt_test.go @@ -17,14 +17,18 @@ import ( // fakeResolver returns a static provider + model. Counts resolve calls for // cache-hit assertions. type fakeResolver struct { - prov providers.Provider - model string - calls atomic.Int32 - resolveErr error + prov providers.Provider + model string + calls atomic.Int32 + resolveErr error + preferredProvider string + preferredModel string } -func (f *fakeResolver) ResolveForHook(_ context.Context, _ uuid.UUID, _ string) (providers.Provider, string, error) { +func (f *fakeResolver) ResolveForHook(_ context.Context, _ uuid.UUID, preferredProvider, preferredModel string) (providers.Provider, string, error) { f.calls.Add(1) + f.preferredProvider = preferredProvider + f.preferredModel = preferredModel if f.resolveErr != nil { return nil, "", f.resolveErr } @@ -115,6 +119,42 @@ func TestPrompt_Allow_StructuredOutput(t *testing.T) { } } +func TestPrompt_ExplicitProviderPassedToResolver(t *testing.T) { + prov := &fakeProvider{nextResp: okResp("allow")} + resolver := &fakeResolver{prov: prov, model: "gpt-5.6-terra"} + h := &handlers.PromptHandler{Resolver: resolver} + cfg := makePromptCfg(t) + cfg.Config["provider"] = "cppai" + cfg.Config["model"] = "gpt-5.6-terra" + + dec, err := h.Execute(context.Background(), cfg, makePromptEv()) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if dec != hooks.DecisionAllow { + t.Fatalf("decision=%q, want allow", dec) + } + if resolver.preferredProvider != "cppai" || resolver.preferredModel != "gpt-5.6-terra" { + t.Fatalf("resolver preferences=(%q, %q), want (cppai, gpt-5.6-terra)", resolver.preferredProvider, resolver.preferredModel) + } +} + +func TestPrompt_ExplicitProviderWithoutModelUsesProviderDefault(t *testing.T) { + prov := &fakeProvider{nextResp: okResp("allow")} + resolver := &fakeResolver{prov: prov, model: "provider-default"} + h := &handlers.PromptHandler{Resolver: resolver, DefaultModel: "haiku"} + cfg := makePromptCfg(t) + cfg.Config["provider"] = "cppai" + delete(cfg.Config, "model") + + if _, err := h.Execute(context.Background(), cfg, makePromptEv()); err != nil { + t.Fatalf("unexpected err: %v", err) + } + if resolver.preferredModel != "" { + t.Fatalf("preferred model=%q, want empty provider-default sentinel", resolver.preferredModel) + } +} + func TestPrompt_Block_StructuredOutput(t *testing.T) { prov := &fakeProvider{nextResp: okResp("block")} h := &handlers.PromptHandler{Resolver: &fakeResolver{prov: prov, model: "m"}} diff --git a/ui/web/src/i18n/locales/en/hooks.json b/ui/web/src/i18n/locales/en/hooks.json index c110d495..6240c622 100644 --- a/ui/web/src/i18n/locales/en/hooks.json +++ b/ui/web/src/i18n/locales/en/hooks.json @@ -72,6 +72,7 @@ "headers": "Headers", "bodyTemplate": "Body Template", "promptTemplate": "Prompt Template", + "provider": "Provider", "model": "Model", "maxInvocationsPerTurn": "Max Invocations per Turn", "scriptSource": "Script source (ES5.1 JavaScript)", @@ -144,6 +145,8 @@ "promptRequiresMatcher": "Prompt hooks require a matcher or if_expr", "invalidRegex": "Invalid regex", "promptTemplateRequired": "Prompt template is required", + "promptProviderRequired": "Provider is required", + "promptModelRequired": "Model is required", "scriptSourceRequired": "Script source is required" }, "decision": { @@ -152,4 +155,4 @@ "error": "Error", "timeout": "Timeout" } -} \ No newline at end of file +} diff --git a/ui/web/src/i18n/locales/ru/hooks.json b/ui/web/src/i18n/locales/ru/hooks.json index ba2dd2f6..b64fa913 100644 --- a/ui/web/src/i18n/locales/ru/hooks.json +++ b/ui/web/src/i18n/locales/ru/hooks.json @@ -72,6 +72,7 @@ "headers": "Заголовки", "bodyTemplate": "Шаблон тела", "promptTemplate": "Шаблон промпта", + "provider": "Провайдер", "model": "Модель", "maxInvocationsPerTurn": "Макс. вызовов за ход", "scriptSource": "Исходный код скрипта (ES5.1 JavaScript)", @@ -138,6 +139,8 @@ "promptRequiresMatcher": "Хуки prompt требуют матчер или if_expr", "invalidRegex": "Недопустимый regex", "promptTemplateRequired": "Шаблон промпта обязателен", + "promptProviderRequired": "Провайдер обязателен", + "promptModelRequired": "Модель обязательна", "scriptSourceRequired": "Исходный код скрипта обязателен" }, "decision": { diff --git a/ui/web/src/i18n/locales/vi/hooks.json b/ui/web/src/i18n/locales/vi/hooks.json index b2c8e10c..527fa732 100644 --- a/ui/web/src/i18n/locales/vi/hooks.json +++ b/ui/web/src/i18n/locales/vi/hooks.json @@ -72,6 +72,7 @@ "headers": "Headers", "bodyTemplate": "Mẫu body", "promptTemplate": "Mẫu prompt", + "provider": "Nhà cung cấp", "model": "Mô hình", "maxInvocationsPerTurn": "Số lần gọi tối đa mỗi lượt", "scriptSource": "Mã nguồn script (ES5.1 JavaScript)", @@ -144,6 +145,8 @@ "promptRequiresMatcher": "Hook prompt yêu cầu matcher hoặc if_expr", "invalidRegex": "Regex không hợp lệ", "promptTemplateRequired": "Mẫu prompt là bắt buộc", + "promptProviderRequired": "Nhà cung cấp là bắt buộc", + "promptModelRequired": "Mô hình là bắt buộc", "scriptSourceRequired": "Mã nguồn script là bắt buộc" }, "decision": { @@ -152,4 +155,4 @@ "error": "Lỗi", "timeout": "Hết thời gian" } -} \ No newline at end of file +} diff --git a/ui/web/src/i18n/locales/zh/hooks.json b/ui/web/src/i18n/locales/zh/hooks.json index c66d64e1..ea39e69f 100644 --- a/ui/web/src/i18n/locales/zh/hooks.json +++ b/ui/web/src/i18n/locales/zh/hooks.json @@ -72,6 +72,7 @@ "headers": "请求头", "bodyTemplate": "请求体模板", "promptTemplate": "Prompt 模板", + "provider": "提供商", "model": "模型", "maxInvocationsPerTurn": "每轮最大调用次数", "scriptSource": "脚本源码（ES5.1 JavaScript）", @@ -144,6 +145,8 @@ "promptRequiresMatcher": "Prompt 钩子需要 matcher 或 if_expr", "invalidRegex": "无效的正则表达式", "promptTemplateRequired": "Prompt 模板为必填项", + "promptProviderRequired": "提供商为必填项", + "promptModelRequired": "模型为必填项", "scriptSourceRequired": "脚本源码为必填项" }, "decision": { @@ -152,4 +155,4 @@ "error": "错误", "timeout": "超时" } -} \ No newline at end of file +} diff --git a/ui/web/src/pages/agents/agent-detail/agent-hooks-tab.tsx b/ui/web/src/pages/agents/agent-detail/agent-hooks-tab.tsx index 7686891d..1146d779 100644 --- a/ui/web/src/pages/agents/agent-detail/agent-hooks-tab.tsx +++ b/ui/web/src/pages/agents/agent-detail/agent-hooks-tab.tsx @@ -15,6 +15,7 @@ import { import { HookListRow } from "@/pages/hooks/components/hook-list-row"; import { HookFormDialog } from "@/pages/hooks/components/hook-form-dialog"; import { HookTestPanel } from "@/pages/hooks/components/hook-test-panel"; +import { buildHookConfig } from "@/pages/hooks/hook-form-config"; import type { HookFormData } from "@/schemas/hooks.schema"; interface AgentHooksTabProps { @@ -23,42 +24,6 @@ interface AgentHooksTabProps { onCreateOpenChange?: (open: boolean) => void; } -function parseHeaders(raw: string | undefined): Record { - const trimmed = (raw ?? "").trim(); - if (!trimmed) return {}; - try { - const parsed = JSON.parse(trimmed); - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - return parsed as Record; - } - throw new Error("headers must be a JSON object"); - } catch (err) { - // eslint-disable-next-line preserve-caught-error -- JSON.parse error message already captured verbatim in thrown message - throw new Error( - "Invalid headers JSON: " + (err instanceof Error ? err.message : String(err)), - ); - } -} - -function buildConfig(data: HookFormData): Record { - if (data.handler_type === "http") { - return { - url: data.url ?? "", - method: data.method ?? "POST", - headers: parseHeaders(data.headers), - body_template: data.body_template ?? "", - }; - } - if (data.handler_type === "script") { - return { source: data.script_source ?? "" }; - } - return { - prompt_template: data.prompt_template ?? "", - model: data.model ?? "haiku", - max_invocations_per_turn: data.max_invocations_per_turn ?? 5, - }; -} - export function AgentHooksTab({ agentId, initialCreateOpen, onCreateOpenChange }: AgentHooksTabProps) { const { t } = useTranslation("agents"); const { t: th } = useTranslation("hooks"); @@ -89,7 +54,7 @@ export function AgentHooksTab({ agentId, initialCreateOpen, onCreateOpenChange } const handleCreate = async (data: HookFormData) => { let config: Record; try { - config = buildConfig(data); + config = buildHookConfig(data); } catch (err) { toast.error(err instanceof Error ? err.message : String(err)); return; @@ -118,7 +83,7 @@ export function AgentHooksTab({ agentId, initialCreateOpen, onCreateOpenChange } if (!editTarget) return; let config: Record; try { - config = buildConfig(data); + config = buildHookConfig(data); } catch (err) { toast.error(err instanceof Error ? err.message : String(err)); return; diff --git a/ui/web/src/pages/hooks/__tests__/hooks-page.test.ts b/ui/web/src/pages/hooks/__tests__/hooks-page.test.ts index a8363519..c1da51cc 100644 --- a/ui/web/src/pages/hooks/__tests__/hooks-page.test.ts +++ b/ui/web/src/pages/hooks/__tests__/hooks-page.test.ts @@ -6,6 +6,7 @@ */ import { describe, it, expect } from "vitest"; import { hookFormSchema } from "@/schemas/hooks.schema"; +import { buildHookConfig } from "@/pages/hooks/hook-form-config"; // --- Zod schema validation --- @@ -33,6 +34,8 @@ describe("hookFormSchema — base cases", () => { handler_type: "prompt", matcher: "^bash$", prompt_template: "Evaluate the tool call.", + provider: "cppai", + model: "gpt-5.6-terra", }); expect(result.success).toBe(true); }); @@ -42,6 +45,8 @@ describe("hookFormSchema — base cases", () => { ...base, handler_type: "prompt", prompt_template: "Evaluate the tool call.", + provider: "cppai", + model: "gpt-5.6-terra", matcher: "", if_expr: "", }); @@ -57,6 +62,8 @@ describe("hookFormSchema — base cases", () => { ...base, handler_type: "prompt", matcher: "^bash$", + provider: "cppai", + model: "gpt-5.6-terra", }); expect(result.success).toBe(false); if (!result.success) { @@ -65,6 +72,21 @@ describe("hookFormSchema — base cases", () => { } }); + it("rejects prompt hook without provider or model", () => { + const result = hookFormSchema.safeParse({ + ...base, + handler_type: "prompt", + matcher: "^bash$", + prompt_template: "Evaluate the tool call.", + }); + expect(result.success).toBe(false); + if (!result.success) { + const paths = result.error.issues.map((e) => e.path.join(".")); + expect(paths).toContain("provider"); + expect(paths).toContain("model"); + } + }); + it("rejects invalid regex in matcher", () => { const result = hookFormSchema.safeParse({ ...base, @@ -132,6 +154,17 @@ describe("hooks i18n key contracts", () => { } }); + it("prompt provider/model keys exist in all locales", async () => { + for (const locale of ["en", "vi", "zh", "ru"]) { + const mod = await import(`@/i18n/locales/${locale}/hooks.json`); + const data = mod as unknown as Record>; + expect(data.form?.provider).toBeTruthy(); + expect(data.form?.model).toBeTruthy(); + expect(data.validation?.promptProviderRequired).toBeTruthy(); + expect(data.validation?.promptModelRequired).toBeTruthy(); + } + }); + it("toast keys exist in en", async () => { const en = await import("@/i18n/locales/en/hooks.json"); const data = en as unknown as Record>; @@ -144,14 +177,24 @@ describe("hooks i18n key contracts", () => { // --- buildConfig helper (re-tested inline) --- describe("hook config builder logic", () => { - it("prompt type config has prompt_template and model", () => { - const config = { + it("prompt type config persists provider and model", () => { + const config = buildHookConfig({ + event: "pre_tool_use", + handler_type: "prompt", + scope: "tenant", + matcher: "^exec$", + timeout_ms: 5000, + on_timeout: "block", + priority: 100, + enabled: true, prompt_template: "Evaluate this.", - model: "haiku", + provider: "cppai", + model: "gpt-5.6-terra", max_invocations_per_turn: 5, - }; + }); expect(config.prompt_template).toBeTruthy(); - expect(config.model).toBe("haiku"); + expect(config.provider).toBe("cppai"); + expect(config.model).toBe("gpt-5.6-terra"); expect(config.max_invocations_per_turn).toBe(5); }); 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 935297a2..fa6add55 100644 --- a/ui/web/src/pages/hooks/components/hook-form-dialog.tsx +++ b/ui/web/src/pages/hooks/components/hook-form-dialog.tsx @@ -16,6 +16,7 @@ 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 { ProviderModelSelect } from "@/components/shared/provider-model-select"; import { ScriptEditor } from "./script-editor"; const HOOK_EVENTS = [ @@ -39,7 +40,7 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo : (["tenant", "agent"] as const); const { - register, control, handleSubmit, watch, reset, + register, control, handleSubmit, watch, reset, setValue, formState: { errors, isSubmitting }, } = useForm({ resolver: zodResolver(hookFormSchema), @@ -54,12 +55,16 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo priority: 100, enabled: true, method: "POST", + provider: "", + model: "", max_invocations_per_turn: 5, }, }); const handlerType = watch("handler_type"); const scope = watch("scope"); + const promptProvider = watch("provider") ?? ""; + const promptModel = watch("model") ?? ""; 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 @@ -94,6 +99,7 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo headers: cfg.headers ? JSON.stringify(cfg.headers) : "", body_template: (cfg.body_template as string) ?? "", prompt_template: (cfg.prompt_template as string) ?? "", + provider: (cfg.provider as string) ?? "", model: (cfg.model as string) ?? "", max_invocations_per_turn: (cfg.max_invocations_per_turn as number) ?? 5, script_source: (cfg.source as string) ?? "", @@ -326,19 +332,29 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo )}
- - ( - - )} /> + { + setValue("provider", value, { shouldDirty: true, shouldValidate: true }); + setValue("model", "", { shouldDirty: true, shouldValidate: true }); + }} + model={promptModel} + onModelChange={(value) => setValue("model", value, { shouldDirty: true, shouldValidate: true })} + providerLabel={t("form.provider")} + modelLabel={t("form.model")} + showVerify + disabled={isBuiltin} + /> + {errors.provider && ( +

+ {t(errors.provider.message ?? "validation.promptProviderRequired")} +

+ )} + {errors.model && ( +

+ {t(errors.model.message ?? "validation.promptModelRequired")} +

+ )}
diff --git a/ui/web/src/pages/hooks/components/hook-overview-tab.tsx b/ui/web/src/pages/hooks/components/hook-overview-tab.tsx index 5e183cbd..87bbf576 100644 --- a/ui/web/src/pages/hooks/components/hook-overview-tab.tsx +++ b/ui/web/src/pages/hooks/components/hook-overview-tab.tsx @@ -163,6 +163,7 @@ function HttpConfigCard({ cfg, t }: { cfg: Record; t: TFn }) { function PromptConfigCard({ cfg, t }: { cfg: Record; t: TFn }) { const promptTemplate = typeof cfg.prompt_template === "string" ? cfg.prompt_template : ""; + const provider = typeof cfg.provider === "string" ? cfg.provider : ""; const model = typeof cfg.model === "string" ? cfg.model : ""; return (
@@ -173,10 +174,17 @@ function PromptConfigCard({ cfg, t }: { cfg: Record; t: TFn }) 
{promptTemplate}
)} - {model && ( + {(provider || model) && (
-

{t("form.model")}

- {model} +

+ {provider && model + ? `${t("form.provider")} / ${t("form.model")}` + : t(provider ? "form.provider" : "form.model")} +

+
+ {provider && {provider}} + {model && {model}} +
)}
diff --git a/ui/web/src/pages/hooks/hook-form-config.ts b/ui/web/src/pages/hooks/hook-form-config.ts new file mode 100644 index 00000000..d5606daa --- /dev/null +++ b/ui/web/src/pages/hooks/hook-form-config.ts @@ -0,0 +1,39 @@ +import type { HookFormData } from "@/schemas/hooks.schema"; + +function parseHeaders(raw: string | undefined): Record { + const trimmed = (raw ?? "").trim(); + if (!trimmed) return {}; + try { + const parsed = JSON.parse(trimmed); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return parsed as Record; + } + throw new Error("headers must be a JSON object"); + } catch (err) { + // The original parser message is included verbatim in the user-facing error. + // eslint-disable-next-line preserve-caught-error + throw new Error( + "Invalid headers JSON: " + (err instanceof Error ? err.message : String(err)), + ); + } +} + +export function buildHookConfig(data: HookFormData): Record { + if (data.handler_type === "http") { + return { + url: data.url ?? "", + method: data.method ?? "POST", + headers: parseHeaders(data.headers), + body_template: data.body_template ?? "", + }; + } + if (data.handler_type === "script") { + return { source: data.script_source ?? "" }; + } + return { + prompt_template: data.prompt_template ?? "", + provider: data.provider ?? "", + model: data.model ?? "", + max_invocations_per_turn: data.max_invocations_per_turn ?? 5, + }; +} diff --git a/ui/web/src/pages/hooks/hooks-page.tsx b/ui/web/src/pages/hooks/hooks-page.tsx index a1281546..3d9e505e 100644 --- a/ui/web/src/pages/hooks/hooks-page.tsx +++ b/ui/web/src/pages/hooks/hooks-page.tsx @@ -23,6 +23,7 @@ import { HookTestPanel } from "./components/hook-test-panel"; import { HookOverviewTab } from "./components/hook-overview-tab"; import { HookHistoryTable } from "./components/hook-history-table"; import { BetaInfoCard } from "./components/beta-info-card"; +import { buildHookConfig } from "./hook-form-config"; import type { HookFormData } from "@/schemas/hooks.schema"; const HOOK_EVENTS = [ @@ -30,47 +31,6 @@ const HOOK_EVENTS = [ "post_tool_use", "post_model_response", "stop", "subagent_start", "subagent_stop", ] as const; -// parseHeaders accepts an empty string, an empty object string, or a JSON -// object. Returns {} for empty/whitespace-only input. Throws a typed Error -// with a friendly message on malformed JSON so the caller can surface via toast. -function parseHeaders(raw: string | undefined): Record { - const trimmed = (raw ?? "").trim(); - if (!trimmed) return {}; - try { - const parsed = JSON.parse(trimmed); - if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { - return parsed as Record; - } - throw new Error("headers must be a JSON object"); - } catch (err) { - // eslint-disable-next-line preserve-caught-error -- JSON.parse error message already captured verbatim in thrown message - throw new Error( - "Invalid headers JSON: " + (err instanceof Error ? err.message : String(err)), - ); - } -} - -function buildConfig(data: HookFormData): Record { - if (data.handler_type === "http") { - return { - url: data.url ?? "", - method: data.method ?? "POST", - headers: parseHeaders(data.headers), - body_template: data.body_template ?? "", - }; - } - if (data.handler_type === "script") { - // Backend goja handler reads cfg.Config.source (Phase 03). Zod caps at 32 KiB. - return { source: data.script_source ?? "" }; - } - // prompt - return { - prompt_template: data.prompt_template ?? "", - model: data.model ?? "haiku", - max_invocations_per_turn: data.max_invocations_per_turn ?? 5, - }; -} - export function HooksPage() { // Route params — single source of truth (CLAUDE.md) const { id: detailId } = useParams<{ id: string }>(); @@ -114,7 +74,7 @@ export function HooksPage() { const handleCreate = async (data: HookFormData) => { let config: Record; try { - config = buildConfig(data); + config = buildHookConfig(data); } catch (err) { toast.error(err instanceof Error ? err.message : String(err)); return; @@ -139,7 +99,7 @@ export function HooksPage() { if (!editTarget) return; let config: Record; try { - config = buildConfig(data); + config = buildHookConfig(data); } catch (err) { toast.error(err instanceof Error ? err.message : String(err)); return; diff --git a/ui/web/src/schemas/hooks.schema.ts b/ui/web/src/schemas/hooks.schema.ts index bd748ca5..dc88d9b0 100644 --- a/ui/web/src/schemas/hooks.schema.ts +++ b/ui/web/src/schemas/hooks.schema.ts @@ -38,6 +38,7 @@ export const hookFormSchema = z headers: z.string().optional(), // JSON string body_template: z.string().optional(), prompt_template: z.string().optional(), + provider: z.string().optional(), model: z.string().optional(), max_invocations_per_turn: z.number().int().min(1).max(20).optional(), // Script handler source (ES5.1 JavaScript). Cap mirrors backend 32 KiB @@ -73,6 +74,20 @@ export const hookFormSchema = z message: "validation.promptTemplateRequired", }); } + if (!data.provider?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["provider"], + message: "validation.promptProviderRequired", + }); + } + if (!data.model?.trim()) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ["model"], + message: "validation.promptModelRequired", + }); + } } if (data.handler_type === "script" && !data.script_source?.trim()) { ctx.addIssue({