fix(web): refresh the project list when a Discover run ends (#808)

.changeset/discover-refresh-on-completion.md

@@ -0,0 +1,16 @@
+---
+"@paddock/web": patch
+---
+
+Discover: imported projects now appear without a manual browser reload (#808)
+
+The first-run import run refreshes the project list the moment it finishes, so
+the sidebar matches the success screen's own claim that "they are in the sidebar
+now" — previously the list was refetched only by the **Get started** button, and
+anyone who read the result and looked at the sidebar saw an instance that still
+said "No projects yet".
+
+Home's empty-instance decision is now latched for the life of the mount and
+released by **Get started**, so that refresh cannot unmount the screen reporting
+the run: the success headline and every per-row outcome — including the rows that
+failed and have something to say about it — stay on screen until the user leaves.

packages/web/src/components/DiscoverView.test.tsx

@@ -75,7 +75,7 @@ function sessionList(over: Partial<DiscoverSessions> = {}): DiscoverSessions {
   };
 }
 
-function renderView(props: { firstRun?: boolean } = {}) {
+function renderView(props: { firstRun?: boolean; onLeave?: () => void } = {}) {
   return render(
     <MemoryRouter>
       <DiscoverView {...props} />
@@ -243,6 +243,28 @@ describe("DiscoverView", () => {
     expect(navigate).toHaveBeenCalledWith("/");
   });
 
+  it("refreshes the project list as soon as the run ends, not only on Get started (#808)", async () => {
+    // The success screen says "They are in the sidebar now", and until this
+    // refresh that sentence was false — the sidebar still read "No projects yet"
+    // and a manual browser reload was the only way to see the import land.
+    const user = userEvent.setup();
+    renderView();
+    await user.click(await screen.findByRole("button", { name: /Import 1 project/ }));
+    await screen.findByRole("button", { name: "Get started" });
+    expect(refresh).toHaveBeenCalledTimes(1);
+  });
+
+  it("tells its host to re-ask rather than relying on navigation (#808)", async () => {
+    // On the Home mount `navigate("/")` cannot end this screen — it already IS
+    // "/". `RootHome` passes its `recheck` in, and that is what hands over.
+    const user = userEvent.setup();
+    const onLeave = vi.fn();
+    renderView({ firstRun: true, onLeave });
+    await user.click(await screen.findByRole("button", { name: /Import 1 project/ }));
+    await user.click(await screen.findByRole("button", { name: "Get started" }));
+    await waitFor(() => expect(onLeave).toHaveBeenCalled());
+  });
+
   it("warns about a divergent recorded path BEFORE the import, not after", async () => {
     discover.mockResolvedValue(
       result({ candidates: [candidate({ recordedPath: "/private/home/ed/code/paddock" })] }),

packages/web/src/components/DiscoverView.tsx

@@ -63,8 +63,31 @@ import {
  * the opposite problem and gets the exclusion tally plus the two toggles that
  * would relax it. Rendering a blank table for either is how you get told the
  * feature is broken.
+ *
+ * ## Refreshing without unmounting yourself (#808)
+ *
+ * On the Home mount this component exists only while the instance is empty, so
+ * the very act of importing removes its own reason to be on screen. The run
+ * therefore refreshes the project list ONCE, on completion, and `RootHome`
+ * latches its empty-instance decision so that refresh cannot pull the success
+ * screen — and every per-row outcome the user still has to read — out from under
+ * them. Deferring the refresh all the way to "Get started" was the original
+ * answer, and it is what the bug was: the finished screen claims the projects
+ * are in the sidebar while the sidebar still says there are none.
  */
-export function DiscoverView({ firstRun = false }: { firstRun?: boolean }) {
+export function DiscoverView({
+  firstRun = false,
+  onLeave,
+}: {
+  firstRun?: boolean;
+  /**
+   * The Home mount's way out. `RootHome` passes its `recheck` here, because a
+   * screen rendered INSTEAD of Home cannot leave by navigating to Home — it has
+   * to tell Home to ask again. The `/discover` route passes nothing: it is an
+   * ordinary route, and `navigate` is enough.
+   */
+  onLeave?: () => void;
+}) {
   const navigate = useNavigate();
   const { refresh } = useProjects();
   const shell = useOutletContext<ShellOutletContext | null>();
@@ -153,20 +176,31 @@ export function DiscoverView({ firstRun = false }: { firstRun?: boolean }) {
     });
     setSubmitting(false);
     setFinished(true);
-  }, [accepted, rows, submitting]);
+    // The rest of the app learns about the new projects HERE, once the run is
+    // over — not per row (that is the mid-run unmount this screen must not
+    // suffer) and not only on "Get started". The success screen says "They are
+    // in the sidebar now", and until this call that sentence was false: the
+    // sidebar still read "No projects yet" and a manual browser reload was the
+    // only way out (#808). Safe to do now only because `RootHome` latches its
+    // empty-instance decision — see {@link useInstanceEmpty}.
+    await refresh();
+  }, [accepted, rows, submitting, refresh]);
 
   /**
    * Leave for Home.
    *
-   * The project list is refreshed HERE rather than after each row, on purpose:
-   * on the Home mount this component only exists while the instance is empty, so
-   * refreshing mid-run would make the first successful row unmount the screen the
-   * user is watching the rest of the run on.
+   * Two steps, and the second is the one that actually moves on the Home mount:
+   * `navigate("/")` is a no-op when this screen already IS `/`, so what ends it
+   * is `onLeave` — `RootHome`'s `recheck`, releasing the latch that has been
+   * holding this component on screen since the run finished. The refresh is kept
+   * even though {@link submit} already ran one: it is the only thing standing
+   * between a failed background refresh and a Home rendered from a stale list.
    */
   const getStarted = useCallback(async () => {
     await refresh();
+    onLeave?.();
     navigate("/");
-  }, [navigate, refresh]);
+  }, [navigate, refresh, onLeave]);
 
   const imported = Object.values(outcomes).reduce((n, o) => n + o.adopted, 0);
   const createdCount = Object.values(outcomes).filter((o) => o.project).length;

packages/web/src/lib/useInstanceEmpty.test.tsx

@@ -75,4 +75,30 @@ describe("useInstanceEmpty", () => {
     act(() => result.current.recheck());
     await waitFor(() => expect(result.current.empty).toBe(false));
   });
+
+  it("HOLDS its answer when projects appear underneath it (#808)", async () => {
+    // The import run is what makes an empty instance non-empty, so an unlatched
+    // answer would flip the moment Discovery refreshed the project list — taking
+    // the success screen, and every per-row failure the user still has to read,
+    // off the page with it. `projects` stays live; only the ANSWER is pinned.
+    const { result, rerender } = renderHook(() => useInstanceEmpty());
+    await waitFor(() => expect(result.current.empty).toBe(true));
+
+    mockProjects = [makeProject({ slug: "alpha" })];
+    rerender();
+    expect(result.current.empty).toBe(true);
+  });
+
+  it("releases the latch on recheck, with no undecided flicker in between", async () => {
+    // "Get started" is the only caller. It must land on the answer directly:
+    // going back through `null` would blank Home for a frame on the one screen
+    // whose whole tri-state exists to avoid exactly that.
+    const { result, rerender } = renderHook(() => useInstanceEmpty());
+    await waitFor(() => expect(result.current.empty).toBe(true));
+
+    mockProjects = [makeProject({ slug: "alpha" })];
+    rerender();
+    act(() => result.current.recheck());
+    expect(result.current.empty).toBe(false);
+  });
 });

packages/web/src/lib/useInstanceEmpty.ts

@@ -27,6 +27,22 @@ import { ROOT_KEY } from "../routes/ProjectView/urls";
  * Having any project at all settles it, and that is the overwhelmingly common
  * case, so the extra request is issued only by an instance with no projects. A
  * populated instance pays nothing for this hook beyond a render.
+ *
+ * ## The answer is LATCHED, and {@link recheck} is how it is released (#808)
+ *
+ * The instance's emptiness is live data — but Home's *front door* must not be.
+ * Discovery is rendered instead of Home while the instance is empty, and
+ * importing is precisely the act that makes it non-empty, so an unlatched answer
+ * means the import run destroys the screen reporting it: the success headline
+ * and every per-row outcome vanish mid-read, replaced by the root workspace.
+ * That is why the refresh used to be deferred all the way to "Get started", and
+ * deferring it is what left the sidebar stale until a manual browser reload.
+ *
+ * So the question is asked once per mount and the answer held until someone asks
+ * again. `recheck` is that ask, and "Get started" is the only caller — the user
+ * saying they have finished reading. Latching only pins the answer, never the
+ * data: `projects` stays live underneath, which is the whole point (the sidebar
+ * populates the moment the run ends, while this screen stays put).
  */
 export function useInstanceEmpty(): {
   empty: boolean | null;
@@ -36,7 +52,17 @@ export function useInstanceEmpty(): {
   const { projects, loading } = useProjects();
   const [rootHasChats, setRootHasChats] = useState<boolean | null>(null);
   const [nonce, setNonce] = useState(0);
-  const recheck = useCallback(() => setNonce((n) => n + 1), []);
+  /**
+   * Bumping the nonce is not enough on its own: it releases the latch below in
+   * the SAME render that still holds the pre-recheck `rootHasChats`, which would
+   * re-pin the very answer we are trying to discard and never move again. The
+   * effect clears it too, but a frame later — so clear it here, where the two
+   * halves of "forget what you knew" cannot come apart.
+   */
+  const recheck = useCallback(() => {
+    setRootHasChats(null);
+    setNonce((n) => n + 1);
+  }, []);
   const noProjects = !loading && projects.length === 0;
 
   useEffect(() => {
@@ -59,13 +85,27 @@ export function useInstanceEmpty(): {
     };
   }, [noProjects, nonce]);
 
-  const empty = loading
+  const answer = loading
     ? null
     : projects.length > 0
       ? false
       : rootHasChats === null
         ? null
         : !rootHasChats;
 
+  /**
+   * The latch: the first non-null answer of each generation, kept.
+   *
+   * Adjusted during render rather than in an effect, deliberately. An effect
+   * lands a frame late, and this hook's contract is that the undecided state is
+   * `null` — a one-frame `false` before the effect catches up is exactly the
+   * flash the tri-state exists to prevent, on exactly the fresh install it exists
+   * for. React re-runs the component immediately on a set-during-render, so the
+   * committed render is the latched one.
+   */
+  const [latched, setLatched] = useState<{ gen: number; value: boolean } | null>(null);
+  if (answer !== null && latched?.gen !== nonce) setLatched({ gen: nonce, value: answer });
+  const empty = latched?.gen === nonce ? latched.value : null;
+
   return { empty, recheck };
 }