From 7096a8e02c71ea7028e50bc28218a9544c3a425d Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 16 Apr 2026 07:44:08 +0700 Subject: [PATCH] fix(hooks/ui): native script editor + redesigned detail + error toast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User report: hook detail page ugly, script not syntax-highlighted, edit dialog crashes silently with no toast. - Replace react-simple-code-editor (React 19 compat issues — likely cause of the silent edit-dialog crash) with a native textarea + 
 overlay.
  Same UX: caret over highlighted source, scroll-synced. Drops the dep
  entirely and removes a render-path black box. Bundle size went DOWN
  (62 KB → was 65 KB before Phase 06; net ~+1 KB for Prism core).
- Import prismjs/themes/prism-tomorrow.css so token colors actually show.
  Editor surface uses #2d2d2d to match the Tomorrow theme palette.
- Harden HookFormDialog reset against null `initial.config` — previous
  cast threw when the row was inserted with no config payload.
- Redesign HookOverviewTab:
  - Header card with event/handler/scope chips + status pill
  - Metadata grid (timeout/on_timeout/priority/source/version/timestamps)
  - Handler-specific cards: script source rendered via the read-only
    ScriptEditor (full Prism highlight), HTTP shows url/method/body,
    prompt shows template + model. No more raw JSON dump.
  - Extract HttpConfigCard / PromptConfigCard subcomponents to keep
    types narrow at the unknown→string boundary.
- ErrorBoundary now toast.error()s when it catches — user sees the
  failure visually instead of just the generic fallback card. Full stack
  still logs to console for devs.
---
 ui/web/package.json                           |   1 -
 ui/web/pnpm-lock.yaml                         |  17 +-
 .../src/components/shared/error-boundary.tsx  |   6 +
 .../hooks/components/hook-form-dialog.tsx     |   5 +-
 .../hooks/components/hook-overview-tab.tsx    | 203 +++++++++++++-----
 .../pages/hooks/components/script-editor.tsx  | 110 ++++++++--
 6 files changed, 259 insertions(+), 83 deletions(-)

diff --git a/ui/web/package.json b/ui/web/package.json
index 3100368c..b160cece 100644
--- a/ui/web/package.json
+++ b/ui/web/package.json
@@ -45,7 +45,6 @@
     "react-i18next": "^17.0.1",
     "react-markdown": "^10.1.0",
     "react-router": "^7.13.2",
-    "react-simple-code-editor": "^0.14.1",
     "recharts": "^3.8.1",
     "rehype-highlight": "^7.0.2",
     "rehype-katex": "^7.0.1",
diff --git a/ui/web/pnpm-lock.yaml b/ui/web/pnpm-lock.yaml
index 6738c83e..dc8efaae 100644
--- a/ui/web/pnpm-lock.yaml
+++ b/ui/web/pnpm-lock.yaml
@@ -4,6 +4,9 @@ settings:
   autoInstallPeers: true
   excludeLinksFromLockfile: false
 
+overrides:
+  prismjs: ^1.30.0
+
 importers:
 
   .:
@@ -101,9 +104,6 @@ importers:
       react-router:
         specifier: ^7.13.2
         version: 7.13.2(react-dom@19.2.4(react@19.2.4))(react@19.2.4)
-      react-simple-code-editor:
-        specifier: ^0.14.1
-        version: 0.14.1(react-dom@19.2.4(react@19.2.4))(react@19.2.4)
       recharts:
         specifier: ^3.8.1
         version: 3.8.1(@types/react@19.2.14)(react-dom@19.2.4(react@19.2.4))(react-is@19.2.4)(react@19.2.4)(redux@5.0.1)
@@ -2915,12 +2915,6 @@ packages:
       react-dom:
         optional: true
 
-  react-simple-code-editor@0.14.1:
-    resolution: {integrity: sha512-BR5DtNRy+AswWJECyA17qhUDvrrCZ6zXOCfkQY5zSmb96BVUbpVAv03WpcjcwtCwiLbIANx3gebHOcXYn1EHow==}
-    peerDependencies:
-      react: '>=16.8.0'
-      react-dom: '>=16.8.0'
-
   react-style-singleton@2.2.3:
     resolution: {integrity: sha512-b6jSvxvVnyptAiLjbkWLE/lOnR4lfTtDAl+eUC7RZy+QQWc6wRzIV2CE6xBuMmDxc2qIihtDCZD5NPOFl7fRBQ==}
     engines: {node: '>=10'}
@@ -6525,11 +6519,6 @@ snapshots:
     optionalDependencies:
       react-dom: 19.2.4(react@19.2.4)
 
-  react-simple-code-editor@0.14.1(react-dom@19.2.4(react@19.2.4))(react@19.2.4):
-    dependencies:
-      react: 19.2.4
-      react-dom: 19.2.4(react@19.2.4)
-
   react-style-singleton@2.2.3(@types/react@19.2.14)(react@19.2.4):
     dependencies:
       get-nonce: 1.0.1
diff --git a/ui/web/src/components/shared/error-boundary.tsx b/ui/web/src/components/shared/error-boundary.tsx
index 94bbaeda..db03396f 100644
--- a/ui/web/src/components/shared/error-boundary.tsx
+++ b/ui/web/src/components/shared/error-boundary.tsx
@@ -2,6 +2,7 @@ import { Component, Fragment, type ErrorInfo, type ReactNode } from "react";
 import { AlertTriangle, RefreshCw } from "lucide-react";
 import { Button } from "@/components/ui/button";
 import i18n from "@/i18n";
+import { toast } from "@/stores/use-toast-store";
 
 interface Props {
   children: ReactNode;
@@ -22,6 +23,11 @@ export class ErrorBoundary extends Component {
 
   componentDidCatch(error: Error, info: ErrorInfo) {
     console.error("[ErrorBoundary]", error, info.componentStack);
+    // Surface the error name + first line of message via toast so users see a
+    // visual indicator instead of just the fallback card. Full stack stays in
+    // console for devs; toast keeps it short to avoid wall-of-text.
+    const msg = (error.message || "Unknown error").split("\n")[0]?.slice(0, 200);
+    toast.error(error.name || "Render error", msg);
   }
 
   private handleRetry = () => {
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 361153ae..3faebd69 100644
--- a/ui/web/src/pages/hooks/components/hook-form-dialog.tsx
+++ b/ui/web/src/pages/hooks/components/hook-form-dialog.tsx
@@ -64,7 +64,10 @@ export function HookFormDialog({ open, onOpenChange, onSubmit, initial }: HookFo
   useEffect(() => {
     if (open) {
       if (initial) {
-        const cfg = initial.config as Record;
+        // Defensive: backend may emit config as null when a row was inserted
+        // without a config payload. Treat null/undefined as empty so the
+        // property reads below don't crash the edit dialog.
+        const cfg = (initial.config ?? {}) as Record;
         // Legacy `command` rows coerce to `http` in the form — user cannot save as
         // `command` (enum narrowed post-Wave-1). Phase 07 auto-disables them regardless.
         const handlerType: HookFormData["handler_type"] =
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 6159ed78..ccf76b24 100644
--- a/ui/web/src/pages/hooks/components/hook-overview-tab.tsx
+++ b/ui/web/src/pages/hooks/components/hook-overview-tab.tsx
@@ -1,79 +1,178 @@
 import { useTranslation } from "react-i18next";
 import { Badge } from "@/components/ui/badge";
 import type { HookConfig } from "@/hooks/use-hooks";
+import { ScriptEditor } from "./script-editor";
+import { SystemBadge } from "./system-badge";
 
 interface HookOverviewTabProps {
   hook: HookConfig;
 }
 
-function InfoRow({ label, children }: { label: string; children: React.ReactNode }) {
+const HANDLER_COLORS: Record = {
+  command: "bg-slate-100 text-slate-700 dark:bg-slate-800 dark:text-slate-300",
+  http: "bg-indigo-100 text-indigo-700 dark:bg-indigo-900/30 dark:text-indigo-300",
+  prompt: "bg-pink-100 text-pink-700 dark:bg-pink-900/30 dark:text-pink-300",
+  script: "bg-emerald-100 text-emerald-700 dark:bg-emerald-900/30 dark:text-emerald-300",
+};
+
+const EVENT_COLORS: Record = {
+  session_start: "bg-blue-100 text-blue-700 dark:bg-blue-900/30 dark:text-blue-300",
+  user_prompt_submit: "bg-violet-100 text-violet-700 dark:bg-violet-900/30 dark:text-violet-300",
+  pre_tool_use: "bg-amber-100 text-amber-700 dark:bg-amber-900/30 dark:text-amber-300",
+  post_tool_use: "bg-green-100 text-green-700 dark:bg-green-900/30 dark:text-green-300",
+  stop: "bg-red-100 text-red-700 dark:bg-red-900/30 dark:text-red-300",
+  subagent_start: "bg-cyan-100 text-cyan-700 dark:bg-cyan-900/30 dark:text-cyan-300",
+  subagent_stop: "bg-orange-100 text-orange-700 dark:bg-orange-900/30 dark:text-orange-300",
+};
+
+function MetaItem({ label, children }: { label: string; children: React.ReactNode }) {
   return (
-    

-      {label}
-      {children}
+    

+      
{label}

+      
{children}

     

   );
 }
 
 export function HookOverviewTab({ hook }: HookOverviewTabProps) {
   const { t } = useTranslation("hooks");
+  const cfg = hook.config as Record | undefined;
+  const scriptSource = hook.handler_type === "script" ? ((cfg?.source as string) ?? "") : "";
 
   return (
-    

-      

-        
-          {hook.event}
-        
-        
-          {hook.handler_type}
-        
-        
+    

+      {/* Header card — primary identity + status */}
+      

+        

+          
+            {hook.event}
+          
+          
+            {hook.handler_type}
+          
           {hook.scope}
-        
+          {hook.source === "builtin" && }
+          

+          
+            
+            {hook.enabled ? "Enabled" : "Disabled"}
+          
+        

         {hook.matcher && (
-          
-            {hook.matcher}
-          
+          

+            
{t("table.matcher")}

+            {hook.matcher}
+          

         )}
         {hook.if_expr && (
-          
-            {hook.if_expr}
-          
+          

+            
{t("form.ifExpr")}

+            {hook.if_expr}
+          

         )}
-        
-          {hook.timeout_ms}ms
-          {" · "}
-          {t("form.onTimeout")}: {hook.on_timeout}
-        
-        
-          {hook.priority}
-        
-        
-          
-            {hook.enabled ? "Yes" : "No"}
-          
-        
-        
-          {hook.source}
-        
-        
-          v{hook.version}
-        
-        
-          {new Date(hook.created_at).toLocaleString()}
-        
-        
-          {new Date(hook.updated_at).toLocaleString()}
-        
       

 
-      {/* Config preview */}
-      {hook.config && Object.keys(hook.config).length > 0 && (
-        

-          
{t("tabs.config")}

-          
-            {JSON.stringify(hook.config, null, 2)}
-          

+      {/* Metadata grid */}
+      

+        

+          
+            {hook.timeout_ms}ms
+          
+          
+            {hook.on_timeout}
+          
+          
+            {hook.priority}
+          
+          
+            {hook.source}
+          
+          
+            v{hook.version}
+          
+          
+            {new Date(hook.created_at).toLocaleString()}
+          
+          
+            {new Date(hook.updated_at).toLocaleString()}
+          
+        

+      

+
+      {/* Handler-specific content */}
+      {hook.handler_type === "script" && scriptSource && (
+        

+          
{t("form.scriptSource")}

+           {}} readOnly minLines={Math.min(20, scriptSource.split("\n").length + 1)} />
+        

+      )}
+
+      {hook.handler_type === "http" && cfg && }
+      {hook.handler_type === "prompt" && cfg && }
+    

+  );
+}
+
+// Narrow Record values to strings before rendering. JSX
+// won't accept `unknown` directly even when the surrounding `&&` proves
+// truthiness, so we extract typed locals at the boundary.
+type TFn = (key: string) => string;
+
+function HttpConfigCard({ cfg, t }: { cfg: Record; t: TFn }) {
+  const url = typeof cfg.url === "string" ? cfg.url : "";
+  const method = typeof cfg.method === "string" ? cfg.method : "";
+  const bodyTemplate = typeof cfg.body_template === "string" ? cfg.body_template : "";
+  return (
+    

+      
{t("tabs.config")}

+      {url && (
+        

+          
{t("form.url")}

+          {url}
+        

+      )}
+      {method && (
+        

+          
{t("form.method")}

+          {method}
+        

+      )}
+      {bodyTemplate && (
+        

+          
{t("form.bodyTemplate")}

+          
{bodyTemplate}

+        

+      )}
+    

+  );
+}
+
+function PromptConfigCard({ cfg, t }: { cfg: Record; t: TFn }) {
+  const promptTemplate = typeof cfg.prompt_template === "string" ? cfg.prompt_template : "";
+  const model = typeof cfg.model === "string" ? cfg.model : "";
+  return (
+    

+      
{t("tabs.config")}

+      {promptTemplate && (
+        

+          
{t("form.promptTemplate")}

+          
{promptTemplate}

+        

+      )}
+      {model && (
+        

+          
{t("form.model")}

+          {model}
         

       )}
     

diff --git a/ui/web/src/pages/hooks/components/script-editor.tsx b/ui/web/src/pages/hooks/components/script-editor.tsx
index e985705a..2e390a70 100644
--- a/ui/web/src/pages/hooks/components/script-editor.tsx
+++ b/ui/web/src/pages/hooks/components/script-editor.tsx
@@ -1,7 +1,9 @@
-import { useMemo } from "react";
-import Editor from "react-simple-code-editor";
+import { useId, useMemo, useRef, type ChangeEvent, type UIEvent } from "react";
 import Prism from "prismjs";
 import "prismjs/components/prism-javascript";
+// Tomorrow theme matches the dark editor surface defined below; tokens get
+// proper color without requiring app-wide CSS coordination.
+import "prismjs/themes/prism-tomorrow.css";
 import { useTranslation } from "react-i18next";
 
 interface ScriptEditorProps {
@@ -13,6 +15,7 @@ interface ScriptEditorProps {
   // string unchanged.
   error?: string;
   readOnly?: boolean;
+  minLines?: number;
 }
 
 // parseLineCol extracts {line, col} from a goja compile-error string. Format
@@ -25,26 +28,96 @@ export function parseLineCol(err?: string): { line: number; col: number } | null
   return { line: parseInt(m[1], 10), col: parseInt(m[2], 10) };
 }
 
-export function ScriptEditor({ value, onChange, error, readOnly }: ScriptEditorProps) {
+// Minimal native overlay editor: a textarea sits on top of a 
 showing
+// the highlighted source. Keeping this in-house instead of pulling in
+// react-simple-code-editor avoids a React 19 compat surface and lets the
+// component lazy-create Prism grammar without crashing when the language
+// hasn't loaded yet.
+export function ScriptEditor({
+  value,
+  onChange,
+  error,
+  readOnly,
+  minLines = 12,
+}: ScriptEditorProps) {
   const { t } = useTranslation("hooks");
+  const id = useId();
+  const preRef = useRef(null);
+  const taRef = useRef(null);
+
   const loc = useMemo(() => parseLineCol(error), [error]);
 
+  const safeValue = value ?? "";
+  // Keep a trailing newline so the highlighted layer matches textarea height
+  // when the file ends without one (avoids the last line getting clipped).
+  const displayValue =
+    safeValue.length === 0 || safeValue.endsWith("\n") ? safeValue : safeValue + "\n";
+
+  const highlighted = useMemo(() => {
+    const lang = Prism.languages.javascript;
+    if (!lang) return escapeHtml(displayValue);
+    try {
+      return Prism.highlight(displayValue, lang, "javascript");
+    } catch {
+      return escapeHtml(displayValue);
+    }
+  }, [displayValue]);
+
+  const handleScroll = (e: UIEvent) => {
+    if (!preRef.current) return;
+    preRef.current.scrollTop = e.currentTarget.scrollTop;
+    preRef.current.scrollLeft = e.currentTarget.scrollLeft;
+  };
+
+  const handleChange = (e: ChangeEvent) => {
+    if (!readOnly) onChange(e.target.value);
+  };
+
   return (
     

-      
-      

-         {} : onChange}
-          highlight={(code) => {
-            const lang = Prism.languages.javascript;
-            return lang ? Prism.highlight(code, lang, "javascript") : code;
+      
+      

+        
+