From 2811a4a2df6be2dc73b715f2904e0cf815382287 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 18:06:13 -0700 Subject: [PATCH] =?UTF-8?q?fix(tests):=20TaskDetailModal=20renders=20throu?= =?UTF-8?q?gh=20a=20PORTAL=20=E2=80=94=20query=20the=20document,=20not=20c?= =?UTF-8?q?ontainer=20(#2885)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What this clears **30 of the 55 failures** in the dashboard `app:backfill 3/4` shard — all in one file, all reading `expected null to be truthy`. ## The symptom points the wrong way That message reads as *"the modal never rendered"*, and that is how this survived. The file's helpers took the `container` returned by `render()` and asked it for the modal's elements: ```ts const header = container.querySelector("[data-testid='agent-log-model-header']"); ``` `TaskDetailModal` mounts inside `FloatingWindow`, which uses **`createPortal`** — so the modal subtree is attached to `document.body`, **not** beneath the container React handed back. Every `container.querySelector` in the file returns null no matter what renders. ## Probed, not inferred I had already spent one wrong hypothesis on this exact file — the shared `TaskDetailModal.test-helpers.ts` carries a genuinely stale `{ flagEnabled: false, workflows: [] }` fixture of the kind #2833 fixed for `App.test.tsx`, so it looked like the 30-failure lever. Adopting `DEFAULT_BOARD_WORKFLOWS` **changed nothing** (still 30 failed). Reverted. So I instrumented instead: ``` P1_after_tab_click menu=true items=["Live","Feed","Raw","Interventions"] P2_after_select viewer=true header=true empty=false ``` The Activity menu opens, `Raw` selects, and the viewer **and** its model header are both present — via `document`. Only the container-rooted lookup could not see them. **Why the file half-worked:** `screen.getByRole(...)` in the same helpers always succeeded, because `screen` queries the document. That mix of query roots is what made a query-root bug look like a rendering fault. 19 `container.querySelector` call sites converted. ## Scope — deliberately narrow **Only this file.** Eight other `TaskDetailModal` specs use `container.querySelector` too — 95 of them in `attachments-and-tabs` alone — and they **all pass today**, because they render `TaskDetailContent` rather than the portalled modal. Converting green files would be churn with real risk and no red to justify it. ## Evidence | | result | |---|---| | the file | **48/48** (was 30 failed) | | shard `3/4` | **55 → 25** failures | | mutation: rename the `agent-log-model-header` testid in `AgentLogViewer` | **18 failed** | The mutation matters here: the fix is "change what we query", so the risk is assertions that now find *something* and stop being load-bearing. They still observe the real component. `pnpm lint` clean. Test-only; `AgentLogViewer.tsx` restored clean. ## Remaining in this shard `settings-mobile` (17), `NodesView` (2), `MailboxModal` (2), `agent-modals-mobile` (2), `TaskDetailModal.pr-tab` (1), `onboarding-flow` (1). Tracked on #2784, which I have been keeping current with per-lane numbers. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- ...ailModal.models-progress-workflow.test.tsx | 64 +++++++++++++------ 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.models-progress-workflow.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.models-progress-workflow.test.tsx index 8f8e20c290..d9179f0765 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.models-progress-workflow.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.models-progress-workflow.test.tsx @@ -114,12 +114,34 @@ describe("TaskDetailModal", () => { ); } - async function openAgentLogAndExpandModelDetails(container: HTMLElement) { + /* + FNXC:TaskDetailModalTests 2026-07-31-15:10: + QUERY THE DOCUMENT — TaskDetailModal renders through a portal, so `container` is the wrong root. + + This helper took the `container` from `render()` and asked it for + `[data-testid='agent-log-model-header']`. TaskDetailModal mounts inside `FloatingWindow`, which + uses `createPortal`, so the modal subtree is attached to `document.body` and NOT beneath the + container React handed back. Every `document.querySelector` in this file therefore returns null + no matter what renders. + + Probed rather than inferred, because the symptom pointed the wrong way — 30 cases failing on + "expected null to be truthy" reads as "the modal never rendered": + + P1_after_tab_click menu=true items=["Live","Feed","Raw","Interventions"] + P2_after_select viewer=true header=true empty=false + + The Activity menu opens, Raw selects, and the viewer AND its model header are both present — via + `document`. Only the container-rooted lookup could not see them. + + Note `screen.getByRole(...)` calls in the same helper always worked, because `screen` queries the + document. That mix is why this file half-worked and why the failure looked like a render problem. + */ + async function openAgentLogAndExpandModelDetails(_container: HTMLElement) { fireEvent.click(screen.getByRole("button", { name: "Activity" })); selectActivityView("raw-logs"); await waitFor(() => { - const header = container.querySelector("[data-testid='agent-log-model-header']"); + const header = document.querySelector("[data-testid='agent-log-model-header']"); expect(header).toBeTruthy(); }); @@ -128,7 +150,7 @@ describe("TaskDetailModal", () => { fireEvent.click(expandButton); } - return container.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; + return document.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; } it("uses task effective settings success path for Raw Logs model display", async () => { @@ -592,14 +614,14 @@ describe("TaskDetailModal", () => { fireEvent.click(screen.getByRole("button", { name: "Activity" })); selectActivityView("raw-logs"); await waitFor(() => { - const header = container.querySelector("[data-testid='agent-log-model-header']"); + const header = document.querySelector("[data-testid='agent-log-model-header']"); expect(header).toBeTruthy(); }); const expandButton = screen.getByTestId("agent-log-model-expand") as HTMLButtonElement; if (expandButton.getAttribute("aria-expanded") !== "true") { fireEvent.click(expandButton); } - const header = container.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; + const header = document.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; expect(header.textContent).toContain("openai/gpt-4o"); expect(header.textContent).toContain("google/gemini-2.5-pro"); }); @@ -649,14 +671,14 @@ describe("TaskDetailModal", () => { fireEvent.click(screen.getByRole("button", { name: "Activity" })); selectActivityView("raw-logs"); await waitFor(() => { - const header = container.querySelector("[data-testid='agent-log-model-header']"); + const header = document.querySelector("[data-testid='agent-log-model-header']"); expect(header).toBeTruthy(); }); const expandButton = screen.getByTestId("agent-log-model-expand") as HTMLButtonElement; if (expandButton.getAttribute("aria-expanded") !== "true") { fireEvent.click(expandButton); } - const header = container.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; + const header = document.querySelector("[data-testid='agent-log-model-header']") as HTMLElement; expect(header.textContent).toContain("openai/gpt-4.1"); }); @@ -680,7 +702,7 @@ describe("TaskDetailModal", () => { />, ); - expect(container.querySelector(".detail-step-progress")).toBeTruthy(); + expect(document.querySelector(".detail-step-progress")).toBeTruthy(); expect(screen.getByText("Progress")).toBeTruthy(); }); @@ -698,7 +720,7 @@ describe("TaskDetailModal", () => { />, ); - expect(container.querySelector(".detail-step-progress")).toBeTruthy(); + expect(document.querySelector(".detail-step-progress")).toBeTruthy(); expect(screen.getByText("(no steps defined)")).toBeTruthy(); }); @@ -722,7 +744,7 @@ describe("TaskDetailModal", () => { />, ); - const segments = container.querySelectorAll(".step-progress-segment"); + const segments = document.querySelectorAll(".step-progress-segment"); expect(segments).toHaveLength(3); }); @@ -747,7 +769,7 @@ describe("TaskDetailModal", () => { />, ); - const segments = container.querySelectorAll(".step-progress-segment"); + const segments = document.querySelectorAll(".step-progress-segment"); expect(segments[0].classList.contains("step-progress-segment--done")).toBe(true); expect(segments[1].classList.contains("step-progress-segment--in-progress")).toBe(true); expect(segments[2].classList.contains("step-progress-segment--pending")).toBe(true); @@ -778,10 +800,10 @@ describe("TaskDetailModal", () => { />, ); - const segments = container.querySelectorAll(".step-progress-segment"); + const segments = document.querySelectorAll(".step-progress-segment"); // 2 impl steps + 2 enabled workflow steps = 4 segments. expect(segments).toHaveLength(4); - const workflowSegments = container.querySelectorAll(".step-progress-segment--source-workflow"); + const workflowSegments = document.querySelectorAll(".step-progress-segment--source-workflow"); expect(workflowSegments).toHaveLength(2); // code-review ran (passed → unified "done"); browser-verification enabled-not-run (pending). expect(segments[2].classList.contains("step-progress-segment--done")).toBe(true); @@ -810,7 +832,7 @@ describe("TaskDetailModal", () => { />, ); - const segments = container.querySelectorAll(".step-progress-segment"); + const segments = document.querySelectorAll(".step-progress-segment"); expect((segments[0] as HTMLElement).style.backgroundColor).toBe("var(--color-success)"); expect((segments[1] as HTMLElement).style.backgroundColor).toBe("var(--in-progress)"); expect((segments[2] as HTMLElement).style.backgroundColor).toBe("var(--border)"); @@ -882,7 +904,7 @@ describe("TaskDetailModal", () => { />, ); - const segments = container.querySelectorAll(".step-progress-segment"); + const segments = document.querySelectorAll(".step-progress-segment"); expect(segments[0].getAttribute("data-tooltip")).toBe("Initialize project (done)"); expect(segments[1].getAttribute("data-tooltip")).toBe("Add tests (in-progress)"); }); @@ -907,14 +929,14 @@ describe("TaskDetailModal", () => { ); // Should be visible in Definition tab - expect(container.querySelector(".detail-step-progress")).toBeTruthy(); + expect(document.querySelector(".detail-step-progress")).toBeTruthy(); // Switch to Activity tab, then Raw Logs segment fireEvent.click(screen.getByRole("button", { name: "Activity" })); selectActivityView("raw-logs"); // Should not be visible in Raw Logs segment - expect(container.querySelector(".detail-step-progress")).toBeNull(); + expect(document.querySelector(".detail-step-progress")).toBeNull(); }); it("step progress is hidden in Comments tab", () => { @@ -940,7 +962,7 @@ describe("TaskDetailModal", () => { fireEvent.click(screen.getByText("Comments")); // Should not be visible in Comments tab - expect(container.querySelector(".detail-step-progress")).toBeNull(); + expect(document.querySelector(".detail-step-progress")).toBeNull(); }); }); @@ -964,7 +986,7 @@ describe("TaskDetailModal", () => { />, ); expect(screen.queryByText("Commits")).toBeNull(); - const tabTexts = Array.from(container.querySelectorAll(".detail-tab")).map((t) => t.textContent); + const tabTexts = Array.from(document.querySelectorAll(".detail-tab")).map((t) => t.textContent); expect(tabTexts).toContain("Changes"); }); }); @@ -1453,14 +1475,14 @@ describe("TaskDetailModal", () => { ); // Definition content visible initially - expect(container.querySelector(".markdown-body")).toBeTruthy(); + expect(document.querySelector(".markdown-body")).toBeTruthy(); // Switch to Workflow tab fireEvent.click(screen.getByText("Workflow")); // Definition content should be hidden await waitFor(() => { - expect(container.querySelector(".markdown-body")).toBeNull(); + expect(document.querySelector(".markdown-body")).toBeNull(); }); }); });