feat(hook): UI improve disconnect error popover

[Bo-Onyx]

Description

Summary

• Unreachable popover: when is_reachable === false, shows up to 3 most recent errors from the last 30 days with timestamps (previously no error details were shown)
• Post-validate refresh: after "Test Connection", the hook card re-fetches the latest state so is_reachable updates immediately in the UI
• Hover-only copy button: CopyIconButton on each error row (popover + logs modal) is hidden until hovering that row, using Hoverable.Root /Hoverable.Item

Motivation: We want to support customer to inject function into certain point in our pipeline.
Eng Doc: https://docs.google.com/document/d/1wCQ4jcuscDLBIuVwzG8yT6UnHVWgIi5gdteOhe1SqhU/edit?tab=t.0
Linear: https://linear.app/onyx-app/project/hooks-14fc5597dc91/overview
UI mocks:
https://www.figma.com/design/sNcHyrXBLtTFDK0ijjMnSk/Admin-Panel?node-id=4756-763808&p=f&m=dev

How Has This Been Tested?

Additional Options

• [Optional] Please cherry-pick this PR to the latest release version.
• [Optional] Override Linear Check

[github-actions]

Preview Deployment

Status	Preview	Commit	Updated
✅	https://onyx-preview-k3uepli1r-danswer.vercel.app	084d875	2026-04-02 22:36:44 UTC

[greptile-apps]

Greptile Summary

This PR improves the hook disconnect error popover by (1) surfacing up to 3 most-recent errors from the last 30 days when is_reachable === false, (2) adding a hover-only copy button using Hoverable.Root/Hoverable.Item in both the popover and logs modal, and (3) refreshing the hook card immediately after "Test Connection" by calling the new getHook service function.

Key changes:

• HookStatusPopover.tsx – Extracts a reusable ErrorLogRow component (eliminating the previously flagged JSX duplication), adds an is_reachable === false branch that merges recentErrors + olderErrors, sorts by recency, and displays the top 3. Timestamp display upgraded from formatTimeOnly to formatDateTimeLog.
• HookLogsModal.tsx – Wraps each LogRow in Hoverable.Root/Hoverable.Item so the copy button is hidden until the row is hovered.
• index.tsx – After validateHook succeeds (or returns a non-passing status), calls getHook to pull the latest is_reachable state and propagates it via onToggled. However, setIsBusy(false) fires in the finally block before getHook completes, briefly re-enabling the button during the refresh fetch.
• svc.ts – Adds getHook(id) using the same parseError pattern as other service functions.

Confidence Score: 4/5

Safe to merge after fixing the isBusy timing issue in handleValidate.

One P1 defect: the finally block in handleValidate clears isBusy before getHook completes, creating a window where the Validate button can be clicked again mid-refresh. All other changes (ErrorLogRow extraction, Hoverable integration, getHook service function) are well-structured and correct.

web/src/ee/refresh-pages/admin/HooksPage/index.tsx — handleValidate isBusy lifecycle

Important Files Changed

Filename	Overview
web/src/ee/refresh-pages/admin/HooksPage/index.tsx	Adds post-validate refresh via getHook, but finally block clears isBusy before getHook completes, leaving a window where the Validate button is re-enabled prematurely.
web/src/ee/refresh-pages/admin/HooksPage/HookStatusPopover.tsx	Extracts ErrorLogRow component (reducing previous duplication), adds is_reachable=false branch showing top-3 errors from last 30 days, switches to formatDateTimeLog for richer timestamps.
web/src/ee/refresh-pages/admin/HooksPage/HookLogsModal.tsx	Wraps LogRow with Hoverable.Root/Hoverable.Item to hide CopyIconButton until hover; group prop derived from created_at+idx for per-row isolation.
web/src/ee/refresh-pages/admin/HooksPage/svc.ts	Adds getHook(id) function — straightforward GET with consistent error parsing, matches the pattern of existing svc functions.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant C as ConnectedHookCard
    participant API as Backend API

    U->>C: Click "Test Connection"
    C->>C: setIsBusy(true)
    C->>API: POST /api/admin/hooks/{id}/validate
    API-->>C: HookValidateResponse
    C->>C: toast (success or error)
    Note over C: finally: setIsBusy(false) ← button re-enabled HERE
    C->>API: GET /api/admin/hooks/{id}
    API-->>C: HookResponse (updated is_reachable)
    C->>C: onToggled(updated) → refresh card state

Loading

Comments Outside Diff (1)

1. web/src/ee/refresh-pages/admin/HooksPage/index.tsx, line 307-332 (link)

   isBusy cleared before getHook completes

   The finally block sets setIsBusy(false) before the second try block runs getHook. Since finally executes unconditionally after the first try/catch, there is a window between the validation response and the refresh response where isBusy is false and the "Test Connection" button becomes clickable again. Any click during that window starts a second overlapping validation.

   The fix is to nest the refresh call inside the first try so the single finally covers both operations:

   async function handleValidate() {
     setIsBusy(true);
     try {
       const result = await validateHook(hook.id);
       if (result.status === "passed") {
         toast.success("Hook validated successfully.");
       } else {
         toast.error(
           result.error_message ?? `Validation failed: ${result.status}`
         );
       }
       try {
         const updated = await getHook(hook.id);
         onToggled(updated);
       } catch (refreshErr) {
         console.error("Failed to refresh hook after validation:", refreshErr);
       }
     } catch (err) {
       console.error("Failed to validate hook:", err);
       toast.error(
         err instanceof Error ? err.message : "Failed to validate hook."
       );
     } finally {
       setIsBusy(false);
     }
   }

   This keeps the early-exit-on-error behaviour without prematurely re-enabling the button.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: web/src/ee/refresh-pages/admin/HooksPage/index.tsx
   Line: 307-332
   
   Comment:
   **`isBusy` cleared before `getHook` completes**
   
   The `finally` block sets `setIsBusy(false)` before the second `try` block runs `getHook`. Since `finally` executes unconditionally after the first `try/catch`, there is a window between the validation response and the refresh response where `isBusy` is `false` and the "Test Connection" button becomes clickable again. Any click during that window starts a second overlapping validation.
   
   The fix is to nest the refresh call inside the first `try` so the single `finally` covers both operations:
   
   ```typescript
   async function handleValidate() {
     setIsBusy(true);
     try {
       const result = await validateHook(hook.id);
       if (result.status === "passed") {
         toast.success("Hook validated successfully.");
       } else {
         toast.error(
           result.error_message ?? `Validation failed: ${result.status}`
         );
       }
       try {
         const updated = await getHook(hook.id);
         onToggled(updated);
       } catch (refreshErr) {
         console.error("Failed to refresh hook after validation:", refreshErr);
       }
     } catch (err) {
       console.error("Failed to validate hook:", err);
       toast.error(
         err instanceof Error ? err.message : "Failed to validate hook."
       );
     } finally {
       setIsBusy(false);
     }
   }
   ```
   
   This keeps the early-exit-on-error behaviour without prematurely re-enabling the button.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: web/src/ee/refresh-pages/admin/HooksPage/index.tsx
Line: 307-332

Comment:
**`isBusy` cleared before `getHook` completes**

The `finally` block sets `setIsBusy(false)` before the second `try` block runs `getHook`. Since `finally` executes unconditionally after the first `try/catch`, there is a window between the validation response and the refresh response where `isBusy` is `false` and the "Test Connection" button becomes clickable again. Any click during that window starts a second overlapping validation.

The fix is to nest the refresh call inside the first `try` so the single `finally` covers both operations:

```typescript
async function handleValidate() {
  setIsBusy(true);
  try {
    const result = await validateHook(hook.id);
    if (result.status === "passed") {
      toast.success("Hook validated successfully.");
    } else {
      toast.error(
        result.error_message ?? `Validation failed: ${result.status}`
      );
    }
    try {
      const updated = await getHook(hook.id);
      onToggled(updated);
    } catch (refreshErr) {
      console.error("Failed to refresh hook after validation:", refreshErr);
    }
  } catch (err) {
    console.error("Failed to validate hook:", err);
    toast.error(
      err instanceof Error ? err.message : "Failed to validate hook."
    );
  } finally {
    setIsBusy(false);
  }
}
```

This keeps the early-exit-on-error behaviour without prematurely re-enabling the button.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "feat(hook): improve disconnect error pop..." | Re-trigger Greptile}

[cubic-dev-ai]

2 issues found across 4 files

Confidence score: 4/5

• This PR looks safe to merge; the noted issues are low severity and mostly around error handling/UI messaging.
• In web/src/ee/refresh-pages/admin/HooksPage/index.tsx, a refresh failure can be reported as a validation failure, which may confuse users with misleading/duplicate toasts.
• Pay close attention to web/src/ee/refresh-pages/admin/HooksPage/index.tsx and web/src/ee/refresh-pages/admin/HooksPage/HookStatusPopover.tsx - error handling and rendering duplication could cause UX drift.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="web/src/ee/refresh-pages/admin/HooksPage/index.tsx">

<violation number="1" location="web/src/ee/refresh-pages/admin/HooksPage/index.tsx:318">
P2: `getHook` refresh is coupled to validation in one catch path, so a refresh failure is reported as a validation failure and can show misleading/duplicate toasts.</violation>
</file>

<file name="web/src/ee/refresh-pages/admin/HooksPage/HookStatusPopover.tsx">

<violation number="1" location="web/src/ee/refresh-pages/admin/HooksPage/HookStatusPopover.tsx:221">
P2: The new unreachable-state branch duplicates the same error-row rendering logic used in the recent-errors branch; extract a shared row/list renderer to avoid drift.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[github-actions]

🖼️ Visual Regression Report

Project	Changed	Added	Removed	Unchanged	Report
admin	0	0	0	165	✅ No changes
exclusive	0	0	0	8	✅ No changes