From c48a736df1e2c2b692bca0fe381888e4ee2d1d47 Mon Sep 17 00:00:00 2001
From: BigSimmo <BigSimmo@users.noreply.github.com>
Date: Sun, 18 Oct 2026 02:13:17 +0800
Subject: [PATCH 1/2] fix(ui): keep Sheet close handlers current before passive
 effects flush

The Close button, swipe and back handlers read onCloseRef/requestCloseRef, which
were updated in a passive useEffect. A click landing after a commit but before
the passive flush called the previous render's onClose. AdminRenewedSheet hit
this in CI (admin-renewed-sheet.dom.test.tsx 'clears the form once a save has
actually completed'): Close ran the pre-save handleClose and skipped the reset.
Update the refs in a layout effect; add a deterministic regression test that
fails without the fix.
---
 src/components/ui/sheet.tsx |  7 ++++-
 tests/sheet.dom.test.tsx    | 53 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/src/components/ui/sheet.tsx b/src/components/ui/sheet.tsx
index 2e60c7f72b..ef42e4cf08 100644
--- a/src/components/ui/sheet.tsx
+++ b/src/components/ui/sheet.tsx
@@ -4,6 +4,7 @@ import {
   useEffect,
   useCallback,
   useId,
+  useLayoutEffect,
   useRef,
   type CSSProperties,
   type PointerEvent as ReactPointerEvent,
@@ -197,7 +198,11 @@ export function Sheet({
     onClose();
   }, [dismissOnBack, onClose, sheetId]);
 
-  useEffect(() => {
+  // Layout effect, not a passive one: the Close button, swipe and back handlers
+  // read these refs, and a click that lands after a commit but before passive
+  // effects flush would otherwise call the previous render's onClose (a stale
+  // closure — e.g. a sheet that had just saved would skip its own reset).
+  useLayoutEffect(() => {
     onCloseRef.current = onClose;
     requestCloseRef.current = requestClose;
   }, [onClose, requestClose]);
diff --git a/tests/sheet.dom.test.tsx b/tests/sheet.dom.test.tsx
index 197805101e..55d4a94e5b 100644
--- a/tests/sheet.dom.test.tsx
+++ b/tests/sheet.dom.test.tsx
@@ -1,4 +1,4 @@
-import { useRef, useState } from "react";
+import { useEffect, useRef, useState } from "react";
 
 import { render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
@@ -325,3 +325,54 @@ describe("Sheet stacked-overlay coordination", () => {
     await waitFor(() => expect(screen.getByRole("button", { name: "Updated return target" })).toHaveFocus());
   });
 });
+
+describe("Sheet close handler freshness", () => {
+  it("calls the latest onClose when Close is clicked right after a commit, before passive effects flush", async () => {
+    // Reproduces the admin-renewed-sheet flake: an async save re-renders the
+    // sheet with a new onClose, and a click that lands between that commit and
+    // the passive-effect flush must not reach the previous render's handler.
+    const { createRoot } = await import("react-dom/client");
+    const previousActEnvironment = (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT;
+    (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = false;
+    const calls: number[] = [];
+    const bump: { current: (() => void) | null } = { current: null };
+    function Harness() {
+      const [n, setN] = useState(0);
+      useEffect(() => {
+        bump.current = () => setN(1);
+      }, []);
+      return (
+        <Sheet open onClose={() => calls.push(n)} title="Freshness sheet" dismissOnBack={false}>
+          <p data-testid="freshness-n">{n}</p>
+        </Sheet>
+      );
+    }
+    const container = document.createElement("div");
+    document.body.appendChild(container);
+    const root = createRoot(container);
+    try {
+      root.render(<Harness />);
+      for (let i = 0; i < 50 && !document.querySelector('[data-testid="freshness-n"]'); i += 1) {
+        await new Promise((resolve) => setTimeout(resolve, 10));
+      }
+      const label = document.querySelector('[data-testid="freshness-n"]')!;
+      await new Promise((resolve) => setTimeout(resolve, 50));
+      const clickedAfterCommit = new Promise<void>((resolve) => {
+        const observer = new MutationObserver(() => {
+          if (label.textContent !== "1") return;
+          observer.disconnect();
+          (document.querySelector('button[aria-label="Close"]') as HTMLButtonElement).click();
+          resolve();
+        });
+        observer.observe(label, { childList: true, characterData: true, subtree: true });
+      });
+      setTimeout(() => bump.current?.(), 0);
+      await clickedAfterCommit;
+      expect(calls).toEqual([1]);
+    } finally {
+      root.unmount();
+      container.remove();
+      (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = previousActEnvironment;
+    }
+  });
+});

From a6906d3949934b3ce2e87bf9bb30c6d5749db334 Mon Sep 17 00:00:00 2001
From: "qodo-code-review[bot]"
 <151058649+qodo-code-review[bot]@users.noreply.github.com>
Date: Sat, 17 Oct 2026 18:18:28 +0000
Subject: [PATCH 2/2] fix: Use an isomorphic layout effect for Sheet handlers

---
 src/components/ui/sheet.tsx | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/components/ui/sheet.tsx b/src/components/ui/sheet.tsx
index ef42e4cf08..06c165f4ac 100644
--- a/src/components/ui/sheet.tsx
+++ b/src/components/ui/sheet.tsx
@@ -25,6 +25,8 @@ import {
   type SheetFocusController,
 } from "@/components/ui/sheet-focus";
 
+const useIsomorphicLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect;
+
 export type SheetMobileSize = "content" | "viewport";
 export type SheetMobileHeaderSafeArea = "none" | "padding" | "offset";
 
@@ -202,7 +204,7 @@ export function Sheet({
   // read these refs, and a click that lands after a commit but before passive
   // effects flush would otherwise call the previous render's onClose (a stale
   // closure — e.g. a sheet that had just saved would skip its own reset).
-  useLayoutEffect(() => {
+  useIsomorphicLayoutEffect(() => {
     onCloseRef.current = onClose;
     requestCloseRef.current = requestClose;
   }, [onClose, requestClose]);