fix(ui): experiments auto-recovery dialog leaves UI dimmed and locked after enabling (#9513)
## 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 `<body>` 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 ? <RecoveryPreviewDialog … /> : 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 <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
GitHub
parent
0e21a27301
commit
b49d178c46
|
|
@ -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<HTMLButtonElement>(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<HTMLButtonElement>("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<HTMLButtonElement>(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<HTMLButtonElement>(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<HTMLButtonElement>("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<HTMLButtonElement>(AUTO_RECOVERY_TOGGLE_SELECTOR);
|
||||
expect(enabledToggle?.getAttribute("aria-checked")).toBe("true");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -170,6 +170,11 @@ export function InstanceExperimentalSettings() {
|
|||
const [previewDialogOpen, setPreviewDialogOpen] = useState(false);
|
||||
const [pendingPreview, setPendingPreview] = useState<IssueGraphLivenessAutoRecoveryPreview | null>(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() {
|
|||
</div>
|
||||
</Card>
|
||||
|
||||
<RecoveryPreviewDialog
|
||||
open={previewDialogOpen}
|
||||
onOpenChange={setPreviewDialogOpen}
|
||||
preview={pendingPreview}
|
||||
onEnableOnly={enableOnly}
|
||||
onEnableAndRun={enableAndRun}
|
||||
isPending={recoveryActionPending}
|
||||
/>
|
||||
{previewDialogOpen ? (
|
||||
<RecoveryPreviewDialog
|
||||
open
|
||||
onOpenChange={(open) => {
|
||||
if (!open) {
|
||||
closeRecoveryPreview();
|
||||
}
|
||||
}}
|
||||
preview={pendingPreview}
|
||||
onEnableOnly={enableOnly}
|
||||
onEnableAndRun={enableAndRun}
|
||||
isPending={recoveryActionPending}
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue