From b49d178c465966f5580f22b0a20b251bc5cd0634 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 13 Jul 2026 11:52:59 -0700 Subject: [PATCH] fix(ui): experiments auto-recovery dialog leaves UI dimmed and locked after enabling (#9513) 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 > - Operators tune instance behavior through Settings → Experiments, where experimental features are toggled on and off > - The task graph liveness auto-recovery experiment shows a confirmation dialog (preview of what would be recovered) before it is enabled > - After confirming with "Enable only" or "Enable and run", the dialog's Radix overlay and the `pointer-events: none` body lock were left behind, dimming the page and blocking all interaction until a refresh > - The dialog was unconditionally mounted and only closed inside the mutation's `onSuccess`, so the overlay teardown depended on the mutation outcome and could race or never happen > - This pull request closes the dialog before the mutation fires in both confirm flows, clears the pending preview alongside the open flag, and mounts the dialog conditionally so its overlay fully unmounts > - The benefit is that enabling an experiment behaves like every other settings change: the dialog goes away, the page stays interactive, and errors surface in the page-level error banner instead of a dead UI ## Linked Issues or Issue Description No public GitHub issue exists for this bug; description follows the bug report template. Refs #4587 (the PR that introduced the configurable liveness auto-recovery controls this dialog belongs to). **What happened?** In Settings → Experiments, toggling on "Task graph liveness auto-recovery" and confirming via "Enable only" left the whole UI dimmed and unclickable. The dialog content disappeared, but the modal overlay and the `pointer-events: none` lock on `` remained until a full page refresh. **Expected behavior:** Confirming (or dismissing) the auto-recovery dialog should close it completely and return the page to a fully interactive state, with the toggle reflecting the new setting. **Steps to reproduce:** 1. Open Settings → Experiments. 2. Toggle on "Task graph liveness auto-recovery"; the confirmation dialog with the recovery preview appears. 3. Click "Enable only". 4. The dialog content disappears but the page stays dimmed and nothing is clickable; refreshing restores the UI and shows the setting was applied. **Paperclip version or commit:** master @ 634ae12 · **Deployment mode:** local instance · **Area:** UI only (`ui/src/pages/InstanceExperimentalSettings.tsx`). ## What Changed - Added a `closeRecoveryPreview()` helper that resets both `previewDialogOpen` and `pendingPreview` together, and used it everywhere the dialog closes (confirm flows, run-mutation success, and user dismissal). - "Enable only" and "Enable and run" now close the dialog *before* firing the mutation, so overlay teardown no longer depends on the mutation outcome; mutation errors roll back the optimistic toggle and surface in the existing page-level error banner. - The `RecoveryPreviewDialog` is now conditionally mounted (`previewDialogOpen ? : null`), guaranteeing the Radix overlay and body pointer-events lock are fully removed when closed. - Added a regression test that walks the real flow — toggle on → preview dialog appears → "Enable only" — and asserts the update payload, that the dialog text and `[data-slot="dialog-overlay"]` element are gone, and that the toggle reads enabled. Credit: the implementation commit was authored by Cody — thanks! This PR packages that fix for upstream review. ## Verification - `pnpm vitest run src/pages/InstanceExperimentalSettings.test.tsx` in `ui/` — 17/17 tests pass, including the new regression test. - Independently re-verified beyond the committed assertions: with a temporary assertion (not committed), confirmed `document.body.style.pointerEvents` is `none` while the dialog is open and released after "Enable only" — the actual "can't interact with anything" symptom, not just overlay DOM removal. - Manual check: open Settings → Experiments, toggle the auto-recovery feature, click "Enable only" — the dialog closes, the page stays interactive, and the toggle shows enabled without a refresh. ## Risks - Low risk: change is confined to one page component's dialog lifecycle; no API, schema, or shared-package changes. - Behavioral shift: the dialog now closes immediately on confirm instead of staying open with a pending spinner until the mutation resolves. Errors are still surfaced via the page-level error banner, and the optimistic toggle rolls back on failure. ## Model Used - Implementation commit authored by the AI coding agent "Cody" (Anthropic Claude-based agent). Review, independent verification, and PR preparation by Claude (Anthropic), model ID `claude-fable-5`, extended thinking with tool use (code execution, 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 --------- Co-authored-by: Cody --- .../InstanceExperimentalSettings.test.tsx | 103 +++++++++++++++++- ui/src/pages/InstanceExperimentalSettings.tsx | 34 ++++-- 2 files changed, 125 insertions(+), 12 deletions(-) diff --git a/ui/src/pages/InstanceExperimentalSettings.test.tsx b/ui/src/pages/InstanceExperimentalSettings.test.tsx index 893990e56a..9a6605fd8a 100644 --- a/ui/src/pages/InstanceExperimentalSettings.test.tsx +++ b/ui/src/pages/InstanceExperimentalSettings.test.tsx @@ -3,7 +3,10 @@ import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import type { InstanceExperimentalSettings as InstanceExperimentalSettingsPayload } from "@paperclipai/shared"; +import type { + InstanceExperimentalSettings as InstanceExperimentalSettingsPayload, + IssueGraphLivenessAutoRecoveryPreview, +} from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { InstanceExperimentalSettings } from "./InstanceExperimentalSettings"; @@ -52,6 +55,8 @@ const SERVER_INFO_TOGGLE_SELECTOR = 'button[aria-label="Toggle server info debug view experimental setting"]'; const BUILT_IN_AGENTS_TOGGLE_SELECTOR = 'button[aria-label="Toggle built-in agents experimental setting"]'; +const AUTO_RECOVERY_TOGGLE_SELECTOR = + 'button[aria-label="Toggle task graph liveness auto-recovery"]'; function defaultExperimentalSettings(): InstanceExperimentalSettingsPayload { return { @@ -81,6 +86,18 @@ function defaultExperimentalSettings(): InstanceExperimentalSettingsPayload { }; } +function emptyRecoveryPreview(): IssueGraphLivenessAutoRecoveryPreview { + return { + lookbackHours: 24, + cutoff: "2026-07-12T16:00:00.000Z", + generatedAt: "2026-07-13T16:00:00.000Z", + findings: 0, + recoverableFindings: 0, + skippedOutsideLookback: 0, + items: [], + }; +} + const WORKTREE_RUN_EXECUTION_TOGGLE_SELECTOR = 'button[aria-label="Toggle worktree run execution setting"]'; @@ -426,4 +443,88 @@ describe("InstanceExperimentalSettings — Conference Room Chat card (PAP-11233) }); expect(toggle?.getAttribute("aria-checked")).toBe("true"); }); + + it("removes the auto-recovery confirmation overlay after enabling only", async () => { + mockInstanceSettingsApi.previewIssueGraphLivenessAutoRecovery.mockResolvedValue(emptyRecoveryPreview()); + await renderPage(); + + const toggle = container.querySelector(AUTO_RECOVERY_TOGGLE_SELECTOR); + expect(toggle?.getAttribute("aria-checked")).toBe("false"); + + await act(async () => { + toggle?.click(); + }); + await flushReact(); + + expect(mockInstanceSettingsApi.previewIssueGraphLivenessAutoRecovery).toHaveBeenCalledWith({ + lookbackHours: 24, + }); + expect(document.body.textContent).toContain("Confirm auto-recovery"); + expect(document.body.querySelector('[data-slot="dialog-overlay"]')).not.toBeNull(); + + const enableOnlyButton = [...document.body.querySelectorAll("button")].find( + (button) => button.textContent === "Enable only", + ); + + await act(async () => { + enableOnlyButton?.click(); + }); + await flushReact(); + + expect(mockInstanceSettingsApi.updateExperimental).toHaveBeenCalledWith({ + enableIssueGraphLivenessAutoRecovery: true, + issueGraphLivenessAutoRecoveryLookbackHours: 24, + }); + expect(document.body.textContent).not.toContain("Confirm auto-recovery"); + expect(document.body.querySelector('[data-slot="dialog-overlay"]')).toBeNull(); + const enabledToggle = container.querySelector(AUTO_RECOVERY_TOGGLE_SELECTOR); + expect(enabledToggle?.getAttribute("aria-checked")).toBe("true"); + }); + + it("removes the auto-recovery confirmation overlay after enabling and running", async () => { + mockInstanceSettingsApi.previewIssueGraphLivenessAutoRecovery.mockResolvedValue(emptyRecoveryPreview()); + mockInstanceSettingsApi.runIssueGraphLivenessAutoRecovery.mockResolvedValue({ + findings: 0, + autoRecoveryEnabled: true, + lookbackHours: 24, + cutoff: "2026-07-12T16:00:00.000Z", + escalationsCreated: 0, + existingEscalations: 0, + skipped: 0, + skippedAutoRecoveryDisabled: 0, + }); + await renderPage(); + + const toggle = container.querySelector(AUTO_RECOVERY_TOGGLE_SELECTOR); + expect(toggle?.getAttribute("aria-checked")).toBe("false"); + + await act(async () => { + toggle?.click(); + }); + await flushReact(); + + expect(document.body.textContent).toContain("Confirm auto-recovery"); + expect(document.body.querySelector('[data-slot="dialog-overlay"]')).not.toBeNull(); + + const enableAndRunButton = [...document.body.querySelectorAll("button")].find( + (button) => button.textContent === "Enable", + ); + + await act(async () => { + enableAndRunButton?.click(); + }); + await flushReact(); + + expect(mockInstanceSettingsApi.updateExperimental).toHaveBeenCalledWith({ + enableIssueGraphLivenessAutoRecovery: true, + issueGraphLivenessAutoRecoveryLookbackHours: 24, + }); + expect(mockInstanceSettingsApi.runIssueGraphLivenessAutoRecovery).toHaveBeenCalledWith({ + lookbackHours: 24, + }); + expect(document.body.textContent).not.toContain("Confirm auto-recovery"); + expect(document.body.querySelector('[data-slot="dialog-overlay"]')).toBeNull(); + const enabledToggle = container.querySelector(AUTO_RECOVERY_TOGGLE_SELECTOR); + expect(enabledToggle?.getAttribute("aria-checked")).toBe("true"); + }); }); diff --git a/ui/src/pages/InstanceExperimentalSettings.tsx b/ui/src/pages/InstanceExperimentalSettings.tsx index c3ee757893..b77f205a3d 100644 --- a/ui/src/pages/InstanceExperimentalSettings.tsx +++ b/ui/src/pages/InstanceExperimentalSettings.tsx @@ -170,6 +170,11 @@ export function InstanceExperimentalSettings() { const [previewDialogOpen, setPreviewDialogOpen] = useState(false); const [pendingPreview, setPendingPreview] = useState(null); + function closeRecoveryPreview() { + setPreviewDialogOpen(false); + setPendingPreview(null); + } + useEffect(() => { setBreadcrumbs([ { label: "Settings", href: "/company/settings" }, @@ -239,7 +244,7 @@ export function InstanceExperimentalSettings() { instanceSettingsApi.runIssueGraphLivenessAutoRecovery({ lookbackHours }), onSuccess: async () => { setActionError(null); - setPreviewDialogOpen(false); + closeRecoveryPreview(); await Promise.all([ queryClient.invalidateQueries({ queryKey: queryKeys.instance.experimentalSettings }), queryClient.invalidateQueries({ queryKey: queryKeys.health }), @@ -310,21 +315,22 @@ export function InstanceExperimentalSettings() { setActionError("Lookback hours must be a whole number from 1 to 720."); return; } + closeRecoveryPreview(); previewMutation.mutate(parsedLookbackHours); } function enableOnly() { if (!lookbackHoursIsValid) return; + closeRecoveryPreview(); toggleMutation.mutate({ enableIssueGraphLivenessAutoRecovery: true, issueGraphLivenessAutoRecoveryLookbackHours: parsedLookbackHours, - }, { - onSuccess: () => setPreviewDialogOpen(false), }); } function enableAndRun() { if (!lookbackHoursIsValid) return; + closeRecoveryPreview(); toggleMutation.mutate({ enableIssueGraphLivenessAutoRecovery: true, issueGraphLivenessAutoRecoveryLookbackHours: parsedLookbackHours, @@ -784,14 +790,20 @@ export function InstanceExperimentalSettings() { - + {previewDialogOpen ? ( + { + if (!open) { + closeRecoveryPreview(); + } + }} + preview={pendingPreview} + onEnableOnly={enableOnly} + onEnableAndRun={enableAndRun} + isPending={recoveryActionPending} + /> + ) : null} ); }