From f77b8a288e8667a70382a82000409b4e4a43c9c3 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 22:36:40 -0600 Subject: [PATCH] fix(ui): keep the GitHub star prompt off tasks that need a Retry (#20713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A task that fails shows a banner with a Retry button. The user clicks Retry to run the task again. > - Self-hosted boards also show a "You completed 5 tasks! Star on GitHub" modal (#20453). > - The modal can open on the task page while the task shows its Retry banner. > - The modal is a Radix dialog. It sets `aria-hidden="true"` on the rest of the page and sets `pointer-events: none` on the body. The Retry button is visible, but a click does not reach it and assistive technology cannot find it. > - This pull request makes the star prompt wait for a calm moment. It does not open on a task or agent chat page, and it does not open while a task Retry or status banner is on screen. > - The benefit is that a visible Retry is always reachable. ## Linked Issues or Issue Description Refs #20453 **What happened?** A restart chaos run left a task in the failed state with the Retry banner. The "You completed 5 tasks!" star modal opened over the same page. The Retry button was in the DOM and visible. Playwright `getByRole('button', { name: 'Retry' })` did not find it, because the modal had set `aria-hidden="true"` on the banner. The body also had `pointer-events: none`, so a user click on Retry did not reach the button. **Expected behavior** A promotional prompt never covers a task that needs the user. A visible Retry is always reachable. **Steps to reproduce** 1. On a self-hosted instance, finish five tasks in one company. Do not open the star prompt before. 2. Let a sixth task fail so that it shows "Couldn't finish this attempt" with Retry. 3. Open that task. The star modal opens over the Retry banner. **Paperclip version or commit** master at 200c6e40e2 ## What Changed - `GitHubStarPrompt` waits while the location is one task (`/:prefix/issues/:ref`) or one agent chat (`/:prefix/chats/:agent`). Task list views (`issues/all`, `issues/done`, and so on) do not count as one task. - `GitHubStarPrompt` waits while a task Retry (`data-testid="task-execution-retry"`) or a task status banner (`role="status"` with `aria-label="Task status"`) is on screen. It uses the same DOM observer that already waits for other dialogs. - If the prompt is open and the user goes back to a task or chat page, or a task Retry or status banner appears, the prompt closes. - `useModalOpen` in `AnnouncementWell.tsx` takes an optional selector. The default is the existing modal selector, so other callers do not change. - New tests in `GitHubStarPrompt.test.tsx`: the prompt stays closed while a Retry banner is on screen and the banner is not `aria-hidden`; the prompt stays closed on a task and a chat page and opens on the task list. The prompt closes when the user goes back to a task, and when a Retry appears. ## Verification - Red first: the two new tests failed on master. A temporary probe in the red run showed the prompt open, the banner with `aria-hidden="true"`, and `document.body.style.pointerEvents === "none"`. - Green: `cd ui && npx vitest run src/components/GitHubStarPrompt.test.tsx src/components/BoardDialogs.test.tsx` passes. - `pnpm verify` passes (481 UI test files). - `pnpm check:token-gates`: all gates clean. - `npx tsc --noEmit -p ui`: no errors in the changed files. ## Risks - Low risk. The prompt can show later than before, because it waits for a non-task page. It still shows only once per browser. - The "seen" marker is written only when the prompt opens, so a user who stays on task pages does not lose the prompt. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), in Claude Code with tool use (shell, file edit, test runs). ## 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 (e.g. `docs/...`, `fix/...`) 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude --- ui/src/components/AnnouncementWell.tsx | 6 +-- ui/src/components/GitHubStarPrompt.test.tsx | 54 ++++++++++++++++++++- ui/src/components/GitHubStarPrompt.tsx | 21 +++++++- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/ui/src/components/AnnouncementWell.tsx b/ui/src/components/AnnouncementWell.tsx index c6212de9fb7..2eb996dd33c 100644 --- a/ui/src/components/AnnouncementWell.tsx +++ b/ui/src/components/AnnouncementWell.tsx @@ -14,16 +14,16 @@ import { AnnouncementCard } from "./AnnouncementCard"; // Includes the command palette, sheets and dialogs created outside DialogContext. export const MODAL_SELECTOR = '[role="dialog"][data-state="open"], [role="alertdialog"][data-state="open"], [aria-modal="true"]:not([data-state="closed"]), dialog[open]'; -export function useModalOpen(enabled: boolean) { +export function useModalOpen(enabled: boolean, selector = MODAL_SELECTOR) { const [open, setOpen] = useState(false); useEffect(() => { if (!enabled) return; - const scan = () => setOpen(Boolean(document.querySelector(MODAL_SELECTOR))); + const scan = () => setOpen(Boolean(document.querySelector(selector))); scan(); const observer = new MutationObserver(scan); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ["data-state", "aria-modal", "open"] }); return () => observer.disconnect(); - }, [enabled]); + }, [enabled, selector]); return open; } diff --git a/ui/src/components/GitHubStarPrompt.test.tsx b/ui/src/components/GitHubStarPrompt.test.tsx index 8da3c90fa54..223f4a849f2 100644 --- a/ui/src/components/GitHubStarPrompt.test.tsx +++ b/ui/src/components/GitHubStarPrompt.test.tsx @@ -2,6 +2,7 @@ import { act } from "react"; import { createRoot, type Root } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { MemoryRouter, useNavigate, type NavigateFunction } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ONBOARDING_FIRST_TASK_ORIGIN_KIND } from "@paperclipai/shared"; import type { HealthStatus } from "@/api/health"; @@ -22,10 +23,14 @@ vi.mock("@/api/issues", () => ({ issuesApi: { describe("GitHub star prompt", () => { let container: HTMLDivElement; let root: Root; - const render = async (health: Partial | null = { deploymentMode: "local_trusted" }) => { + let navigate: NavigateFunction | null = null; + function NavigateHandle() { navigate = useNavigate(); return null; } + const render = async (health: Partial | null = { deploymentMode: "local_trusted" }, path = "/CHA/dashboard") => { const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); if (health) client.setQueryData(["health"], health); - await act(async () => root.render()); + await act(async () => root.render( + + )); // Let the onboarding and done-count queries settle, however slow the run. for (let idle = 0, tick = 0; idle < 2 && tick < 100; tick++) { await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); @@ -65,6 +70,51 @@ describe("GitHub star prompt", () => { expect(visible()).toBe(true); }); + it("never covers a task's Retry: it waits until no Retry banner is on screen", async () => { + const banner = document.createElement("div"); + banner.setAttribute("role", "status"); banner.setAttribute("aria-label", "Task status"); + banner.innerHTML = 'Couldn’t finish this attempt. Your message is saved.'; + document.body.append(banner); + try { + await render(); + expect(visible()).toBe(false); + expect(banner.closest('[aria-hidden="true"]')).toBeNull(); + await act(async () => { banner.remove(); }); + expect(visible()).toBe(true); + } finally { banner.remove(); } + }); + + it("does not interrupt while the user is looking at a task or a chat", async () => { + await render(undefined, "/CHA/issues/CHA-4"); + expect(visible()).toBe(false); + await act(async () => { navigate!("/CHA/chats/claude"); }); + expect(visible()).toBe(false); + expect(window.localStorage.getItem(STAR_PROMPT_SEEN_KEY)).toBeNull(); + await act(async () => { navigate!("/CHA/issues"); }); + expect(visible()).toBe(true); + }); + + it("closes when the user goes back to a task, or a task asks for Retry", async () => { + await render(); + expect(visible()).toBe(true); + await act(async () => { navigate!("/CHA/issues/CHA-4"); }); + expect(visible()).toBe(false); + + window.localStorage.clear(); + await act(async () => root.unmount()); + root = createRoot(container); + await render(); + expect(visible()).toBe(true); + const retry = document.createElement("button"); + retry.setAttribute("data-testid", "task-execution-retry"); + document.body.append(retry); + try { + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + expect(visible()).toBe(false); + expect(retry.closest('[aria-hidden="true"]')).toBeNull(); + } finally { retry.remove(); } + }); + it("does not open after another tab has shown it", async () => { const other = document.createElement("div"); other.setAttribute("role", "dialog"); other.setAttribute("data-state", "open"); diff --git a/ui/src/components/GitHubStarPrompt.tsx b/ui/src/components/GitHubStarPrompt.tsx index 598804a29d8..70edef31f5d 100644 --- a/ui/src/components/GitHubStarPrompt.tsx +++ b/ui/src/components/GitHubStarPrompt.tsx @@ -7,6 +7,7 @@ import { Button } from "@/components/ui/button"; import { Dialog, DialogContent, DialogDescription, DialogHeader, DialogTitle } from "@/components/ui/dialog"; import { useCompany } from "@/context/CompanyContext"; import { useDialogState } from "@/context/DialogContext"; +import { useLocation } from "@/lib/router"; import { queryKeys } from "@/lib/queryKeys"; import { MODAL_SELECTOR, useModalOpen, useOnboardingInProgress } from "./AnnouncementWell"; import { PaperclipMark } from "./PaperclipLockup"; @@ -14,6 +15,15 @@ import { PaperclipMark } from "./PaperclipLockup"; export const STAR_PROMPT_COMPLETED_TASKS = 5; export const STAR_PROMPT_SEEN_KEY = "paperclip.github-star-prompt.seen"; export const PAPERCLIP_GITHUB_URL = "https://github.com/paperclipai/paperclip"; +export const TASK_ATTENTION_SELECTOR = '[data-testid="task-execution-retry"], [role="status"][aria-label="Task status"]'; +const TASK_LIST_VIEWS = new Set(["all", "active", "backlog", "done", "recent"]); + +export function isTaskFocusPath(pathname: string) { + const segments = pathname.split("/").filter(Boolean); + const [section, ref] = segments.slice(-2); + return (segments.length === 2 || segments.length === 3) && Boolean(ref) + && (section === "chats" || (section === "issues" && !TASK_LIST_VIEWS.has(ref!))); +} function readSeen() { try { return window.localStorage.getItem(STAR_PROMPT_SEEN_KEY) !== null; } catch { return true; } @@ -48,6 +58,8 @@ export function GitHubStarPrompt() { }); const reached = eligible && done.isSuccess && done.data.length >= STAR_PROMPT_COMPLETED_TASKS; const otherModalOpen = useModalOpen(reached); + const taskNeedsAttention = useModalOpen(reached || open, TASK_ATTENTION_SELECTOR); + const midTask = isTaskFocusPath(useLocation().pathname); useEffect(() => { if (seen) return; @@ -59,13 +71,18 @@ export function GitHubStarPrompt() { }, [seen]); useEffect(() => { - if (!reached || otherModalOpen || document.querySelector(MODAL_SELECTOR)) return; + if (!reached || otherModalOpen || taskNeedsAttention || midTask + || document.querySelector(`${MODAL_SELECTOR}, ${TASK_ATTENTION_SELECTOR}`)) return; // Another tab may have shown it after this one mounted. if (readSeen()) { setSeen(true); return; } markSeen(); setSeen(true); setOpen(true); - }, [reached, otherModalOpen]); + }, [reached, otherModalOpen, taskNeedsAttention, midTask]); + + useEffect(() => { + if (open && (midTask || taskNeedsAttention)) setOpen(false); + }, [open, midTask, taskNeedsAttention]); return (