fix(web): don't save retention age until committed#22208

@@ -134,20 +192,32 @@ function RetentionControl({
           className="w-auto"
           onValueChange={setDraft}
           onValueCommitted={(next) => {
-            if (next === null) setDraft(value);
-            else {
-              const days = Math.min(3650, Math.max(minimum, Math.round(next)));
-              setDraft(days);
-              onChange(days);
-            }
+            if (next === null) setDraft(value ?? initialDays);
+            else commitDays(next);
           }}
         >
           <NumberFieldGroup>
             <NumberFieldDecrement aria-label={`Decrease ${label}`} />
             <NumberFieldInput
               aria-label={`${label} in days`}
-              size={new Intl.NumberFormat().format(draft ?? value).length}
+              size={new Intl.NumberFormat().format(draft ?? value ?? initialDays).length}
               className="field-sizing-content w-auto min-w-[1ch] grow-0 text-right"
+              onKeyDown={(event) => {
+                if (event.nativeEvent.isComposing) return;
+                if (modeRef.current !== "draft") return;
+                if (event.key === "Escape") {
+                  event.preventDefault();
+                  event.stopPropagation();
+                  discardDraft();
+                  return;
+                }
+                if (event.key !== "Enter") return;
+                event.preventDefault();
+                const raw = event.currentTarget.value.trim();
+                if (raw === "") return;
+                const parsed = Number(raw);
+                if (Number.isFinite(parsed)) commitDays(parsed);

@@ -115,16 +115,74 @@ function RetentionControl({
   onChange: (value: number | null) => void;
   minimum?: number;
 }) {
-  const [draft, setDraft] = useState(value);
+  const initialDays = Math.max(minimum, 8);
+  const [draft, setDraft] = useState<number | null>(value);
   const [savedValue, setSavedValue] = useState(value);
+  // `draft` is an age the user can see and has not saved. `holding` is that age after
+  // commit, kept on screen until the saved value arrives so the row does not flash Off.
+  const [mode, setMode] = useState<"synced" | "draft" | "holding">("synced");
+  const modeRef = useRef(mode);
+  const rootRef = useRef<HTMLDivElement>(null);
+  const pointerInsideRef = useRef(false);
+  const publishedRef = useRef<number | null>(null);
   if (savedValue !== value) {
     setSavedValue(value);
     setDraft(value);
+    setMode("synced");
+  }
+
+  useEffect(() => {
+    modeRef.current = mode;
+    if (mode === "synced") publishedRef.current = null;
+    if (mode !== "draft") return;
+    const input = rootRef.current?.querySelector("input");
+    input?.focus();
+    input?.select();
+  }, [mode]);
+
+  const enabled = value !== null || mode !== "synced";
+
+  function discardDraft() {
+    modeRef.current = "synced";
+    publishedRef.current = null;
+    setMode("synced");
+    setDraft(value);
+  }
+
+  function commitDays(next: number) {
+    const days = Math.min(3650, Math.max(minimum, Math.round(next)));
+    if (value === null && modeRef.current === "holding" && publishedRef.current === days) return;
+    setDraft(days);
+    if (value === null) {
+      modeRef.current = "holding";
+      setMode("holding");
+      publishedRef.current = days;
+    }
+    onChange(days);
   }
 
   return (
-    <div className="flex items-center gap-3">
-      {value !== null ? (
+    <div
+      ref={rootRef}
+      className="flex items-center gap-3"
+      onPointerDown={() => {
+        pointerInsideRef.current = true;
+      }}
+      onPointerUp={() => {
+        pointerInsideRef.current = false;
+      }}
+      onPointerCancel={() => {
+        pointerInsideRef.current = false;
+      }}
+      onBlur={(event) => {
+        if (modeRef.current !== "draft") return;
+        const next = event.relatedTarget;
+        if (next instanceof Node && event.currentTarget.contains(next)) return;
+        if (pointerInsideRef.current) return;

@@ -115,16 +115,74 @@ function RetentionControl({
   onChange: (value: number | null) => void;
   minimum?: number;
 }) {
-  const [draft, setDraft] = useState(value);
+  const initialDays = Math.max(minimum, 8);
+  const [draft, setDraft] = useState<number | null>(value);
   const [savedValue, setSavedValue] = useState(value);
+  // `draft` is an age the user can see and has not saved. `holding` is that age after
+  // commit, kept on screen until the saved value arrives so the row does not flash Off.
+  const [mode, setMode] = useState<"synced" | "draft" | "holding">("synced");
+  const modeRef = useRef(mode);
+  const rootRef = useRef<HTMLDivElement>(null);
+  const pointerInsideRef = useRef(false);
+  const publishedRef = useRef<number | null>(null);
   if (savedValue !== value) {
     setSavedValue(value);
     setDraft(value);
+    setMode("synced");
+  }

@@ -112,19 +112,92 @@ function RetentionControl({
 }: {
   label: string;
   value: number | null;
-  onChange: (value: number | null) => void;
+  onChange: (value: number | null) => Promise<boolean> | void;
   minimum?: number;
 }) {
-  const [draft, setDraft] = useState(value);
+  const initialDays = Math.max(minimum, 8);
+  const [draft, setDraft] = useState<number | null>(value);
   const [savedValue, setSavedValue] = useState(value);
+  // `draft` is an age the user can see and has not saved. `holding` is that age after
+  // commit, kept on screen until the saved value arrives so the row does not flash Off.
+  const [mode, setMode] = useState<"synced" | "draft" | "holding">("synced");
+  const modeRef = useRef(mode);
+  const rootRef = useRef<HTMLDivElement>(null);
+  const pointerInsideRef = useRef(false);
+  const publishedRef = useRef<number | null>(null);
+  const valueRef = useRef(value);
+  valueRef.current = value; // oxlint-disable-line react/refs -- The save result can resolve before effects, and holding must see the latest prop.

@@ -112,19 +120,87 @@ function RetentionControl({
 }: {
   label: string;
   value: number | null;
-  onChange: (value: number | null) => void;
+  onChange: (value: number | null) => Promise<boolean> | void;
   minimum?: number;
 }) {
-  const [draft, setDraft] = useState(value);
+  const initialDays = Math.max(minimum, 8);
+  const [draft, setDraft] = useState<number | null>(value);
   const [savedValue, setSavedValue] = useState(value);
+  // `holding` keeps a committed age visible until the saved value arrives, so the row does not flash Off.
+  const [mode, setMode] = useState<"synced" | "draft" | "holding" | "off">("synced");

@@ -112,19 +120,87 @@ function RetentionControl({
 }: {
   label: string;
   value: number | null;
-  onChange: (value: number | null) => void;
+  onChange: (value: number | null) => Promise<boolean> | void;
   minimum?: number;
 }) {
-  const [draft, setDraft] = useState(value);
+  const initialDays = Math.max(minimum, 8);
+  const [draft, setDraft] = useState<number | null>(value);
   const [savedValue, setSavedValue] = useState(value);
+  // `holding` keeps a committed age visible until the saved value arrives, so the row does not flash Off.
+  const [mode, setMode] = useState<"synced" | "draft" | "holding" | "off">("synced");
+  const modeRef = useRef(mode);
+  const rootRef = useRef<HTMLDivElement>(null);
+  const pointerInsideRef = useRef(false);
+  const publishedRef = useRef<number | null>(null);
+  const offGenRef = useRef(0);
+  const valueRef = useRef(value);
   if (savedValue !== value) {
     setSavedValue(value);
     setDraft(value);
+    // A queued age can arrive after the user already chose Off. Keep that choice until null lands.
+    if (value === null || mode !== "off") setMode("synced");

@@ -0,0 +1,52 @@
+import * as NodeChildProcess from "node:child_process";
+import * as NodeFS from "node:fs";
+import * as NodePath from "node:path";
+const root = NodePath.resolve(import.meta.dirname, "../../..");
+const home = process.argv[2];
+if (!home?.startsWith("/tmp/issue-14743-"))
+  throw new Error("Use a disposable /tmp/issue-14743- home");
+NodeFS.mkdirSync(`${home}/userdata`, { recursive: true });
+const cleanup = {
+  worktreeAfterDays: null,
+  worktreeSettledAfterDays: null,
+  worktreeOnMerge: false,
+  worktreeOnDelete: false,
+  worktreeUnchanged: false,
+  browserArtifactsAfterDays: null,
+  logsAfterDays: null,
+  worktreeKeepWhen: "uncommitted-changes",
+};
+NodeFS.writeFileSync(
+  `${home}/userdata/settings.json`,
+  JSON.stringify({ storageCleanup: cleanup, projectSettingsOverrides: {} }, null, 2),
+);
+NodeFS.mkdirSync(`${home}/project`, { recursive: true });
+NodeChildProcess.execFileSync("git", ["init", "--quiet", `${home}/project`]);
+for (const [name, sha] of [
+  ["before", "22840ab943b8418744b48223411e46ba4fdcef1a"],
+  ["after", "d28e64cfc1a2d17f12681a22b6487fbc05023821"],
+]) {
+  const src = NodeChildProcess.execFileSync(
+    "git",
+    ["show", `${sha}:apps/web/src/components/settings/StorageSettings.tsx`],
+    { cwd: root, encoding: "utf8" },
+  );
+  const control = src.slice(
+    src.indexOf("function RetentionControl("),
+    src.indexOf("const KEEP_WHEN_LABELS"),
+  );
+  const dir = `${root}/apps/web/src/evidence-14743`;
+  NodeFS.mkdirSync(dir, { recursive: true });
+  NodeFS.writeFileSync(
+    `${dir}/${name}.tsx`,
+    `import React,{useState,useRef,useLayoutEffect} from 'react';\nimport {createRoot} from 'react-dom/client';\nimport {Switch} from '../components/ui/switch';\nimport {NumberField,NumberFieldDecrement,NumberFieldGroup,NumberFieldIncrement,NumberFieldInput} from '../components/ui/number-field';\nimport '../index.css';\n${control}\nconst logs=[];let pending=[];let publish;let reset;window.proof={logs,pending,resolve:(id,ok,value)=>{const p=pending.find(p=>p.id===id);if(!p)throw new Error("Unknown pending request "+id);if(p.settled)throw new Error("Already settled request "+id);p.settled=true;logs.push({ack:id,ok,value,at:new Date().toISOString()});if(value!==undefined)publish(value);p.resolve(ok)},broadcast:v=>{logs.push({broadcast:v});publish(v)},reset:()=>reset()};\nfunction App(){const [value,setValue]=useState(null);const [key,setKey]=useState(0);const [mode,setMode]=useState('success');const [log,setLog]=useState([]);publish=setValue;reset=()=>{setValue(null);setKey(k=>k+1);pending.length=0;logs.length=0;setLog([])};return <main className="p-8 space-y-6"><h1>Actual RetentionControl component harness — ${name}</h1><p>Controlled callback outcomes; NOT real scoped runner / persistence proof.</p><p>Source ${sha}; pending requests require explicit acknowledgement.</p><label>Callback mode <select value={mode} onChange={e=>setMode(e.target.value)}><option>success</option><option>pending</option><option>failure</option></select></label><section className="flex items-center justify-between border p-4"><span>Delete inactive worktrees</span><RetentionControl key={key} label="Delete inactive worktrees" value={value} onChange={v=>{const id=logs.filter(x=>'request' in x).length+1;logs.push({request:id,value:v,at:new Date().toISOString()});setLog([...logs]);if(mode==='success'){setValue(v);return Promise.resolve(true)}if(mode==='failure')return Promise.resolve(false);return new Promise(resolve=>pending.push({id,value:v,resolve}))}}/></section><button onClick={()=>reset()}>Reset seed</button><button onClick={()=>setLog([...logs])}>Refresh log</button><button>Outside focus target</button><pre>{JSON.stringify({authoritative:value,requests:log},null,2)}</pre></main>}createRoot(document.getElementById('root')).render(<App/>);`,
+  );
+  NodeFS.writeFileSync(
+    `${root}/apps/web/evidence-14743-${name}.html`,
+    `<html><head><meta name="viewport" content="width=device-width, initial-scale=1.0"></head><body><div id="root"></div><script type="module" src="/src/evidence-14743/${name}.tsx"></script></body></html>`,
+  );

@@ -124,3 +127,152 @@ it("keeps cleanup disabled when settled retention and all other rules are off",
   await act(async () => button.click());
   expect(state.run).not.toHaveBeenCalled();
 });
+
+it("saves a newly enabled retention rule only once its visible age is committed", async () => {
+  const label = "Delete inactive worktrees";
+  setup();
+  await using screen = await render(panel());
+  await act(async () =>
+    screen.container.querySelector<HTMLElement>(`[role="switch"][aria-label="${label}"]`)!.click(),
+  );
+  expect(state.update).not.toHaveBeenCalled();
+
+  const input = screen.container.querySelector<HTMLInputElement>(
+    `input[aria-label="${label} in days"]`,
+  )!;
+  await act(async () => {
+    Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, "30");
+    input.dispatchEvent(new Event("input", { bubbles: true }));
+    input.blur();
+  });
+  expect(state.update).toHaveBeenCalledExactlyOnceWith({
+    storageCleanup: { worktreeAfterDays: 30 },
+  });
+});
+
+const inactive = "Delete inactive worktrees";
+const ruleSwitch = (root: ParentNode, label: string) =>
+  root.querySelector<HTMLElement>(`[role="switch"][aria-label="${label}"]`)!;
+const daysInput = (root: ParentNode, label: string) =>
+  root.querySelector<HTMLInputElement>(`input[aria-label="${label} in days"]`);
+function setInputValue(input: HTMLInputElement, value: string) {
+  Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, value);
+  input.dispatchEvent(new Event("input", { bubbles: true }));
+}
+const pressKey = (input: HTMLInputElement, key: string) =>
+  input.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }));
+async function openDraft(label: string) {
+  const screen = await render(panel());
+  await act(async () => ruleSwitch(screen.container, label).click());
+  return screen;
+}

@@ -124,3 +127,154 @@ it("keeps cleanup disabled when settled retention and all other rules are off",
   await act(async () => button.click());
   expect(state.run).not.toHaveBeenCalled();
 });
+
+it("saves a newly enabled retention rule only once its visible age is committed", async () => {
+  const label = "Delete inactive worktrees";
+  setup();
+  await using screen = await render(panel());
+  await act(async () =>
+    screen.container.querySelector<HTMLElement>(`[role="switch"][aria-label="${label}"]`)!.click(),
+  );
+  expect(state.update).not.toHaveBeenCalled();
+
+  const input = screen.container.querySelector<HTMLInputElement>(
+    `input[aria-label="${label} in days"]`,
+  )!;
+  await act(async () => {
+    Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, "30");
+    input.dispatchEvent(new Event("input", { bubbles: true }));
+    input.blur();
+  });
+  expect(state.update).toHaveBeenCalledExactlyOnceWith({
+    storageCleanup: { worktreeAfterDays: 30 },
+  });
+});
+
+const inactive = "Delete inactive worktrees";
+const ruleSwitch = (root: ParentNode, label: string) =>
+  root.querySelector<HTMLElement>(`[role="switch"][aria-label="${label}"]`)!;
+const daysInput = (root: ParentNode, label: string) =>
+  root.querySelector<HTMLInputElement>(`input[aria-label="${label} in days"]`);
+function setInputValue(input: HTMLInputElement, value: string) {
+  Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!.call(input, value);
+  input.dispatchEvent(new Event("input", { bubbles: true }));
+}
+const pressKey = (input: HTMLInputElement, key: string) =>
+  input.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true }));
+async function openDraft(label: string) {
+  await using stack = new AsyncDisposableStack();
+  const screen = stack.use(await render(panel()));
+  await act(async () => ruleSwitch(screen.container, label).click());
+  stack.move();
+  return screen;
+}
+
+it("saves the visible default when Enter commits an untouched retention draft", async () => {
+  setup();
+  await using screen = await openDraft(inactive);
+  await act(async () => pressKey(daysInput(screen.container, inactive)!, "Enter"));
+  expect(state.update).toHaveBeenCalledExactlyOnceWith({
+    storageCleanup: { worktreeAfterDays: 8 },
+  });
+});
+
+it("does not save a retention draft whose visible text is not a number", async () => {
+  setup();
+  await using screen = await openDraft(inactive);
+  const input = daysInput(screen.container, inactive)!;
+  for (const text of ["+", "."]) {
+    input.select();
+    await userEvent.keyboard(`${text}{Enter}`);
+    expect(input.value).toBe(text);
+  }
+  expect(state.update).not.toHaveBeenCalled();
+  input.select();
+  await userEvent.keyboard("30{Enter}");
+  expect(state.update).toHaveBeenCalledExactlyOnceWith({
+    storageCleanup: { worktreeAfterDays: 30 },
+  });
+});
+
+it("still cancels a retention draft left after clicking its days label", async () => {
+  setup();
+  await using screen = await openDraft(inactive);
+  const input = daysInput(screen.container, inactive)!;
+  await userEvent.click(input.parentElement!.querySelector("span[aria-hidden]")!);
+  const outside = document.createElement("button");
+  document.body.append(outside);
+  await userEvent.click(outside);
+  outside.remove();
+  expect(daysInput(screen.container, inactive)).toBeNull();
+  expect(state.update).not.toHaveBeenCalled();
+});
+
+it("saves a retention step from the drafted age", async () => {
+  const label = "Delete old rotated logs";
+  setup();
+  await using screen = await openDraft(label);
+  const step = screen.container.querySelector<HTMLElement>(`[aria-label="Increase ${label}"]`)!;
+  await act(async () => step.click());
+  expect(state.update).toHaveBeenCalledExactlyOnceWith({ storageCleanup: { logsAfterDays: 9 } });
+});
+
+it("returns a retention rule to off when saving its age fails", async () => {
+  setup();
+  state.update.mockResolvedValue(false);
+  await using screen = await openDraft(inactive);
+  await act(async () => pressKey(daysInput(screen.container, inactive)!, "Enter"));
+  expect(state.update).toHaveBeenCalledOnce();
+  expect(daysInput(screen.container, inactive)).toBeNull();
+});
+
+it("saves an age edited while an earlier enabling save is pending", async () => {
+  setup();
+  let failFirst!: (saved: boolean) => void;
+  state.update
+    .mockReturnValueOnce(new Promise<boolean>((resolve) => (failFirst = resolve)))
+    .mockReturnValueOnce(new Promise<boolean>(() => {}));
+  await using screen = await openDraft(inactive);
+  const input = daysInput(screen.container, inactive)!;
+  await act(async () => pressKey(input, "Enter"));
+  await act(async () => {
+    setInputValue(input, "30");
+    input.blur();
+  });
+  expect(state.update).toHaveBeenNthCalledWith(2, { storageCleanup: { worktreeAfterDays: 30 } });
+
+  await act(async () => failFirst(false));
+  expect(daysInput(screen.container, inactive)?.value).toBe("30");