From ca9c17df605662c735f900b229e9953e30ed4eda Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Sat, 5 Sep 2026 08:40:38 -0700 Subject: [PATCH] test(ui): flush passive effects with React act in CompanySkills tests (#12878) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The user interface tests verify company skill installation flows. > - The install-preview dialog sets state through passive React effects. > - The local test helper flushed render work but did not flush passive effects. > - Timer hops did not guarantee that React completed those effects before user actions. > - This pull request delegates the helper to React act and removes the timer hops. > - The benefit is deterministic CompanySkills tests without a product-code change. ## Linked Issues or Issue Description **What happened?** The CompanySkills install-preview dialog tests used a timer hop after opening the dialog. The timer could run before React completed passive effects. A later click then used stale dialog state. **Expected behavior** The test helper must flush passive effects before the test interacts with the dialog. The tests must pass without a race between timer callbacks and React scheduler tasks. **Steps to reproduce** 1. Run the CompanySkills test file many times from the ui directory. 2. Observe intermittent failures that report an empty agent list or a null slug. 3. Run the same tests with React act to flush passive effects. **Paperclip version or commit** Commit 834f33c31b2734aa6d2d86c7a7d7e1973353a4c1. **Deployment mode** Local dev (pnpm dev). **Installation method** Built from source (pnpm dev / pnpm build). **Agent adapter(s) involved** Not adapter-specific (core bug). **Database mode** Not database-related. ## What Changed - Delegate the local test helper to React act. - Remove four setTimeout(0) hops that no longer provide synchronization. - Keep all 29 tests and all product code unchanged. ## Verification - Run npx vitest run src/pages/CompanySkills.test.tsx from ui/. - Run pnpm --filter @paperclipai/ui exec tsc --noEmit. - Confirm that the full CI suite passes. ## Risks Low risk. The change affects one test helper and one test file. It changes test synchronization only. ## Model Used OpenAI Codex, GPT-5, with tool use and code review support. The exact context window and reasoning configuration are not exposed in this run. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs) - [x] My branch name describes the change (for example test/...) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- ui/src/pages/CompanySkills.test.tsx | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/ui/src/pages/CompanySkills.test.tsx b/ui/src/pages/CompanySkills.test.tsx index 11147e97e7..56c365708d 100644 --- a/ui/src/pages/CompanySkills.test.tsx +++ b/ui/src/pages/CompanySkills.test.tsx @@ -1,7 +1,6 @@ // @vitest-environment jsdom -import type { ComponentProps, ReactNode } from "react"; -import { flushSync } from "react-dom"; +import { act as reactAct, type ComponentProps, type ReactNode } from "react"; import { createRoot, type Root } from "react-dom/client"; import type { CatalogSkill, CompanySkillDetail, CompanySkillListItem, CompanySkillVersion, FolderListResult } from "@paperclipai/shared"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -104,11 +103,9 @@ let root: Root | null = null; let container: HTMLDivElement | null = null; async function act(callback: () => void | Promise) { - let result: void | Promise = undefined; - flushSync(() => { - result = callback(); + await reactAct(async () => { + await callback(); }); - await result; } afterEach(() => { @@ -1043,11 +1040,6 @@ describe("install-time agent enablement", () => { />, ); }); - // The dialog seeds its slug/agent state in passive effects; give them a - // macrotask to flush before interacting. - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 0)); - }); const node = container as ParentNode; expect(node.textContent).toContain("Enable for agents"); @@ -1089,15 +1081,9 @@ describe("install-time agent enablement", () => { // Dialog opens before the agents query resolves: nothing to select yet. await act(async () => renderDialog([])); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 0)); - }); // The agents arrive later; the untouched selection must pick them up. await act(async () => renderDialog(agentOptions)); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 0)); - }); await click(buttonsNamed(container as ParentNode, "Install skill")[0] as HTMLButtonElement); @@ -1129,9 +1115,6 @@ describe("install-time agent enablement", () => { />, ); }); - await act(async () => { - await new Promise((resolve) => setTimeout(resolve, 0)); - }); const node = container as ParentNode; expect(node.textContent).not.toContain("Enable for agents");