From fab36115bcbcc55afcbe0194c9b5c33f04a1c819 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 28 May 2026 21:06:47 -0300 Subject: [PATCH] fix(agent-bridge): default-import Button in RiskNoticeModal to stop prod render crash (R4 #1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Button.tsx` exposes only a default export, but `RiskNoticeModal.tsx` was importing `{ Button }` (named) — so `Button` resolved to `undefined` and every render of the modal crashed with React's "Element type is invalid". The modal opens on first DNS activation for every agent, so this bug effectively broke DNS interception for every agent in production. It went undetected through 3 rounds of code review because two test artifacts masked the failure: 1. `tests/unit/ui/agent-card.test.tsx > calls onDnsToggle when DNS button clicked` failed since round 3 with the exact error "Element type is invalid... Check the render method of RiskNoticeModal", but was repeatedly dismissed as "pre-existing / flaky". 2. `tests/unit/ui/agent-card-risk-modal.test.tsx` (rodada 3) mocked Button as a named export — which made the test green even though the production import was broken. Classic "test alignment to broken code" anti-pattern. This commit: - switches RiskNoticeModal to `import Button from "@/shared/components/Button"` - adds a regression-guard test that source-greps for the default-import shape and asserts Button.tsx remains default-only - updates the named mock in agent-card-risk-modal.test.tsx to `default:` so tests now reflect the real module shape (no more masking) - updates the agent-card.test.tsx DNS-click test to seed the per-agent risk-accepted localStorage flag, isolating the DNS-toggle path from the risk-modal path (the modal flow is covered by the risk-modal spec) After: agent-card.test.tsx 4/4 green; agent-card-risk-modal.test.tsx 5/5 green; new regression guard prevents recurrence of either pattern. --- src/shared/components/RiskNoticeModal.tsx | 2 +- .../risk-notice-modal-button-import.test.ts | 56 +++++++++++++++++++ tests/unit/ui/agent-card-risk-modal.test.tsx | 6 +- tests/unit/ui/agent-card.test.tsx | 7 +++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 tests/unit/risk-notice-modal-button-import.test.ts diff --git a/src/shared/components/RiskNoticeModal.tsx b/src/shared/components/RiskNoticeModal.tsx index 5579cf696c..d668fac36f 100644 --- a/src/shared/components/RiskNoticeModal.tsx +++ b/src/shared/components/RiskNoticeModal.tsx @@ -2,7 +2,7 @@ import { useEffect } from "react"; import { useTranslations } from "next-intl"; -import { Button } from "@/shared/components/Button"; +import Button from "@/shared/components/Button"; export interface RiskNoticeModalProps { open: boolean; diff --git a/tests/unit/risk-notice-modal-button-import.test.ts b/tests/unit/risk-notice-modal-button-import.test.ts new file mode 100644 index 0000000000..f5556a9480 --- /dev/null +++ b/tests/unit/risk-notice-modal-button-import.test.ts @@ -0,0 +1,56 @@ +/** + * Regression guard for the RiskNoticeModal Button import bug (R4 fix #1). + * + * For 3 review rounds the agent-card.test.tsx failure ("Element type is invalid + * — Check the render method of RiskNoticeModal") was misclassified as + * "pre-existing / flaky". The actual root cause was a broken named import: + * + * import { Button } from "@/shared/components/Button"; // ← undefined + * + * `Button.tsx` exposes only a default export, so the named import resolved to + * `undefined`, causing every render of RiskNoticeModal to crash with React's + * "Element type is invalid" error. The modal opens on first DNS activation of + * every agent — so the bug effectively broke DNS interception for every agent + * in production. The R4 reviewer caught it; this test prevents recurrence. + */ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import path from "node:path"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const MODAL = path.resolve(__dirname, "../../src/shared/components/RiskNoticeModal.tsx"); +const BUTTON = path.resolve(__dirname, "../../src/shared/components/Button.tsx"); + +const modalSrc = readFileSync(MODAL, "utf-8"); +const buttonSrc = readFileSync(BUTTON, "utf-8"); + +describe("RiskNoticeModal Button import (R4 fix #1, prod crash regression guard)", () => { + it("Button.tsx exposes a default export", () => { + assert.ok( + /export\s+default\s+function\s+Button/.test(buttonSrc), + "Button.tsx must keep its default export", + ); + }); + + it("Button.tsx does NOT have a named `export { Button }` or `export function Button`", () => { + assert.ok( + !/export\s+(\{[^}]*\bButton\b[^}]*\}|function\s+Button|const\s+Button)/.test( + buttonSrc.replace(/export\s+default\s+function\s+Button/g, ""), + ), + "Button.tsx is default-only — if you add a named export, also fix any default consumers", + ); + }); + + it("RiskNoticeModal imports Button as default (not named)", () => { + assert.ok( + /import\s+Button\s+from\s+["']@\/shared\/components\/Button["']/.test(modalSrc), + "RiskNoticeModal must use `import Button from ...` (default), not `import { Button }`", + ); + assert.ok( + !/import\s+\{\s*Button\s*\}\s+from\s+["']@\/shared\/components\/Button["']/.test(modalSrc), + "Named import of Button from the .tsx file is broken (Button.tsx has no named export)", + ); + }); +}); diff --git a/tests/unit/ui/agent-card-risk-modal.test.tsx b/tests/unit/ui/agent-card-risk-modal.test.tsx index f122411283..a1f1b98521 100644 --- a/tests/unit/ui/agent-card-risk-modal.test.tsx +++ b/tests/unit/ui/agent-card-risk-modal.test.tsx @@ -21,8 +21,12 @@ vi.mock("next/link", () => ({ React.createElement("a", { href }, children), })); +// Button.tsx exposes a default export — match the real module shape so +// RiskNoticeModal (which uses `import Button from ...`) resolves correctly. +// Round 3 had this as a named-export mock, which masked the production +// `import { Button }` bug fixed in R4 #1. vi.mock("@/shared/components/Button", () => ({ - Button: ({ + default: ({ children, onClick, }: { diff --git a/tests/unit/ui/agent-card.test.tsx b/tests/unit/ui/agent-card.test.tsx index 9ea5669eb6..84ae2fb5bc 100644 --- a/tests/unit/ui/agent-card.test.tsx +++ b/tests/unit/ui/agent-card.test.tsx @@ -124,6 +124,13 @@ describe("AgentCard", { timeout: 30000 }, () => { "../../../src/app/(dashboard)/dashboard/tools/agent-bridge/components/AgentCard" ); + // Simulate that the per-agent RiskNoticeModal (Fix4 M5) has already been + // accepted for this agent — otherwise the DNS click opens the modal first + // and onDnsToggle is only called after the user accepts. We test the + // "already accepted" path here; the modal flow is covered by + // tests/unit/ui/agent-card-risk-modal.test.tsx. + localStorage.setItem("omniroute-agentbridge-risk-dismissed-copilot", "true"); + const onDnsToggle = vi.fn().mockResolvedValue(undefined); const container = makeContainer();