fix(onboarding): a failed completion leaves confirm usable

The baseline form raised its pending flag and lowered it on only two of
three exits; the success path went through the confirm screen's finish(),
which caught its own error and returned normally. A failed completion
therefore disabled both buttons until the page was reloaded.

The reset now lives in a finally, finish() rethrows so the form sees the
failure, and the two managed-arm buttons report it themselves. The step
passes before the completion run off a ledger ref, so a retry after a
failure midway no longer re-sends a step the ledger already carries.

src/components/onboarding/__tests__/confirm-failure-recovery.test.ts

@@ -0,0 +1,155 @@
+/**
+ * v1.39 (Wave C, C2) — a failed completion leaves the confirm screen usable.
+ *
+ * Two defects, one symptom. `advance()` in the baseline form set its pending
+ * flag and never reset it on the success path, and `finish()` on the confirm
+ * screen caught its own error and returned normally — so a failed
+ * `POST /api/onboarding/complete` (or a 429 on the answer PATCHes before it)
+ * left "Überspringen" and "Bestätigen und weiter" disabled for the rest of the
+ * page's life and only a reload recovered (research I10).
+ *
+ * The second defect is the retry itself: `finish()` fires one PATCH per
+ * question the flow never showed, and a failure midway left a partly-passed
+ * ledger that the next attempt walked from the top again (M-j).
+ *
+ * Both are pinned at the seam the component drives, because this repo's
+ * component tests render server-side and cannot click.
+ */
+import { describe, expect, it, vi } from "vitest";
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+
+import {
+  defaultOnboardingSteps,
+  emptyOnboardingNeeds,
+  type OnboardingStateDto,
+} from "@/lib/onboarding/needs";
+
+import { runBaselineAttempt } from "../baseline-form";
+import { runConfirmFinish } from "../confirm-screen";
+
+type PassableStep = Parameters<
+  Parameters<typeof runConfirmFinish>[0]["passStep"]
+>[0];
+
+/**
+ * A flow with no unit-bearing area: Q6 is the question the machine never
+ * showed, so the confirm screen has to pass it on the ledger before the
+ * completion.
+ */
+function state(
+  overrides: Partial<OnboardingStateDto> = {},
+): OnboardingStateDto {
+  return {
+    steps: defaultOnboardingSteps().map((step) =>
+      step.id === "who" ? { ...step, status: "done" as const } : step,
+    ),
+    needs: { ...emptyOnboardingNeeds(), recordTarget: "me" },
+    completedAt: null,
+    firstResult: null,
+    ...overrides,
+  };
+}
+
+/** The ledger as the server would answer it after one step was passed. */
+function withPassed(
+  base: OnboardingStateDto,
+  ids: readonly string[],
+): OnboardingStateDto {
+  return {
+    ...base,
+    steps: base.steps.map((step) =>
+      ids.includes(step.id) ? { ...step, status: "skipped" as const } : step,
+    ),
+  };
+}
+
+describe("runBaselineAttempt", () => {
+  it("hands the buttons back when the completion rejects", async () => {
+    const pending: boolean[] = [];
+    const onError = vi.fn();
+    await runBaselineAttempt(
+      (value) => pending.push(value),
+      async () => {
+        throw new Error("complete failed");
+      },
+      onError,
+    );
+    expect(pending).toEqual([true, false]);
+    expect(onError).toHaveBeenCalledTimes(1);
+  });
+
+  it("hands the buttons back on the success path too", async () => {
+    const pending: boolean[] = [];
+    await runBaselineAttempt(
+      (value) => pending.push(value),
+      async () => {},
+      vi.fn(),
+    );
+    expect(pending).toEqual([true, false]);
+  });
+
+  it("is the form's only way to move the pending flag", () => {
+    // A second, inline `setSaving(false)` is how the flag stopped being
+    // reset on one of the three exits in the first place.
+    const source = readFileSync(
+      join(process.cwd(), "src/components/onboarding/baseline-form.tsx"),
+      "utf8",
+    );
+    expect(source).not.toContain("setSaving(false)");
+    expect(source).not.toContain("setSaving(true)");
+  });
+});
+
+describe("runConfirmFinish", () => {
+  it("rethrows a failed completion instead of returning normally", async () => {
+    const ledger = { current: state() };
+    const passed: PassableStep[] = [];
+    const navigate = vi.fn();
+    await expect(
+      runConfirmFinish({
+        ledger,
+        // The server answers the written ledger; only the runner may put it
+        // back, which is the whole point of the ref.
+        passStep: async (step) => {
+          passed.push(step);
+          return withPassed(ledger.current, [step]);
+        },
+        complete: async () => {
+          throw new Error("429");
+        },
+        navigate,
+      }),
+    ).rejects.toThrow("429");
+    // Q6 was never shown for a flow with no unit-bearing area, so it is
+    // passed on the ledger before the completion — and that write survived.
+    expect(passed).toEqual(["units"]);
+    expect(navigate).not.toHaveBeenCalled();
+  });
+
+  it("does not pass a step the failed attempt already passed (M-j)", async () => {
+    const ledger = { current: state() };
+    const passed: PassableStep[] = [];
+    const passStep = async (step: PassableStep) => {
+      passed.push(step);
+      return withPassed(ledger.current, [step]);
+    };
+    let attempt = 0;
+    const complete = async () => {
+      attempt += 1;
+      if (attempt === 1) throw new Error("429");
+      return { onboarding: ledger.current };
+    };
+    const navigate = vi.fn();
+
+    await expect(
+      runConfirmFinish({ ledger, passStep, complete, navigate }),
+    ).rejects.toThrow("429");
+    await runConfirmFinish({ ledger, passStep, complete, navigate });
+
+    // "units" exactly once across both attempts; "first-result" is the
+    // second attempt's own pass, because these answers offer no task.
+    expect(passed).toEqual(["units", "first-result"]);
+    expect(navigate).toHaveBeenCalledTimes(1);
+  });
+});

src/components/onboarding/baseline-form.tsx

@@ -71,6 +71,37 @@ interface BaselineFormState {
   gender: string;
 }
 
+/**
+ * v1.39 (Wave C, C2) — one attempt at confirming, with the buttons always
+ * handed back.
+ *
+ * `advance()` used to raise its own pending flag and lower it again on two of
+ * the three exits: the refused-field return and its catch. The third exit —
+ * the success path, `await onConfirmed()` — never lowered it, and
+ * `onConfirmed` is the confirm screen's completion, which caught its own
+ * error and returned normally. So a failed completion disabled "Skip" and
+ * "Confirm and continue" for the rest of the page's life and only a reload
+ * recovered (research I10).
+ *
+ * The reset lives in a `finally` here, which is the one place it cannot be
+ * forgotten on a new exit, and the component has no other way to move the
+ * flag.
+ */
+export async function runBaselineAttempt(
+  setPending: (pending: boolean) => void,
+  attempt: () => Promise<void>,
+  onError: (err: unknown) => void,
+): Promise<void> {
+  setPending(true);
+  try {
+    await attempt();
+  } catch (err) {
+    onError(err);
+  } finally {
+    setPending(false);
+  }
+}
+
 export function BaselineForm({
   initial,
   backHref,
@@ -152,50 +183,50 @@ export function BaselineForm({
 
   async function advance(opts: { saveProfile: boolean }) {
     if (saving || confirming) return;
-    setSaving(true);
     setFieldErrors({});
-    try {
-      if (opts.saveProfile) {
-        const profileBody = buildBaselineProfileBody(
-          form,
-          heightAdapter.toCanonicalCm(form.height),
-        );
-        if (Object.keys(profileBody).length > 0) {
-          const outcome = describeBaselineSaveOutcome(
-            await putBaselineProfile(profileBody),
-            t,
-            baselineFieldLabelKeys(heightAdapter.usesFeetInches),
+    await runBaselineAttempt(
+      setSaving,
+      async () => {
+        if (opts.saveProfile) {
+          const profileBody = buildBaselineProfileBody(
+            form,
+            heightAdapter.toCanonicalCm(form.height),
           );
-          if (outcome.notice) {
-            const show =
-              outcome.notice.tone === "warning" ? toast.warning : toast.error;
-            show(outcome.notice.message);
+          if (Object.keys(profileBody).length > 0) {
+            const outcome = describeBaselineSaveOutcome(
+              await putBaselineProfile(profileBody),
+              t,
+              baselineFieldLabelKeys(heightAdapter.usesFeetInches),
+            );
+            if (outcome.notice) {
+              const show =
+                outcome.notice.tone === "warning" ? toast.warning : toast.error;
+              show(outcome.notice.message);
+            }
+            if (!outcome.advance) {
+              // A field was refused. Staying on the step is the point —
+              // each refused input now says why under itself, the values
+              // the person typed are still in front of them, and the
+              // account is not stamped as set up over a value that never
+              // landed.
+              setFieldErrors(outcome.fieldErrors);
+              return;
+            }
           }
-          if (!outcome.advance) {
-            // A field was refused. Staying on the step is the point —
-            // each refused input now says why under itself, the values
-            // the person typed are still in front of them, and the
-            // account is not stamped as set up over a value that never
-            // landed.
-            setFieldErrors(outcome.fieldErrors);
-            setSaving(false);
-            return;
-          }
-        }
 
-        // Anamnesis — only write when the user actually filled a field
-        // (the helper returns null for an untouched card, so a
-        // collapsed card never round-trips).
-        const aboutMeBody = buildAnamnesisAboutMeBody(baseAboutMe, anamnesis);
-        if (aboutMeBody) {
-          await apiPut("/api/coach/about-me", aboutMeBody);
+          // Anamnesis — only write when the user actually filled a field
+          // (the helper returns null for an untouched card, so a
+          // collapsed card never round-trips).
+          const aboutMeBody = buildAnamnesisAboutMeBody(baseAboutMe, anamnesis);
+          if (aboutMeBody) {
+            await apiPut("/api/coach/about-me", aboutMeBody);
+          }
         }
-      }
-      await onConfirmed();
-    } catch (err) {
-      toast.error(localizedApiError(err, t, "onboarding.errorGeneric"));
-      setSaving(false);
-    }
+        await onConfirmed();
+      },
+      (err) =>
+        toast.error(localizedApiError(err, t, "onboarding.errorGeneric")),
+    );
   }
 
   return (

src/components/onboarding/confirm-screen.tsx

@@ -1,6 +1,6 @@
 "use client";
 
-import { useState } from "react";
+import { useRef, useState } from "react";
 import Link from "next/link";
 import { useRouter } from "next/navigation";
 import { toast } from "sonner";
@@ -20,7 +20,13 @@ import { localeLabels } from "@/lib/i18n/config";
 import { useTranslations } from "@/lib/i18n/context";
 import { MODULE_REGISTRY } from "@/lib/modules/registry";
 import { confirmedModules } from "@/lib/onboarding/confirm-summary";
-import type { OnboardingStateDto } from "@/lib/onboarding/needs";
+import {
+  ONBOARDING_SKIPPABLE_STEP_IDS,
+  type OnboardingStateDto,
+} from "@/lib/onboarding/needs";
+
+/** A step the confirm screen may pass on the ledger without an answer. */
+type PassableStepId = (typeof ONBOARDING_SKIPPABLE_STEP_IDS)[number];
 import {
   firstResultApplies,
   nextScreen,
@@ -51,6 +57,40 @@ import {
  * offer, otherwise done. When there is no task, the first-result step is
  * passed explicitly so the ledger reads as settled rather than owed.
  */
+/**
+ * v1.39 (Wave C, C2) — the completion sequence, apart from the component so
+ * a failure partway through it can be pinned without a browser.
+ *
+ * Passes every question the flow never showed (the route derives only once
+ * EVERY question is answered or passed, and a step nobody was asked cannot be
+ * answered), completes, passes the first-result step when there is no task to
+ * offer, and navigates. It reports a failure by THROWING — the caller owns the
+ * buttons and the toast.
+ */
+export async function runConfirmFinish(args: {
+  /** The ledger as the last write left it; updated in place as steps pass. */
+  ledger: { current: OnboardingStateDto };
+  managedRecordId?: string;
+  passStep: (step: PassableStepId) => Promise<OnboardingStateDto>;
+  complete: (
+    managedRecordId?: string,
+  ) => Promise<{ onboarding?: OnboardingStateDto | null }>;
+  navigate: (href: string) => void;
+}): Promise<void> {
+  for (const id of questionsToPassBeforeConfirm(args.ledger.current)) {
+    args.ledger.current = await args.passStep(id);
+  }
+  const { onboarding } = await args.complete(args.managedRecordId);
+  let written = onboarding ?? args.ledger.current;
+  if (!firstResultApplies(written)) {
+    // Nothing to offer, so the step is passed rather than left owed: a
+    // pending step would keep the setup reading as unfinished forever.
+    written = await args.passStep("first-result");
+  }
+  args.ledger.current = written;
+  args.navigate(screenHref(nextScreen(written, "confirm") ?? "done"));
+}
+
 export function ConfirmScreen({ state }: { state: OnboardingStateDto }) {
   const { t, locale } = useTranslations();
   const router = useRouter();
@@ -58,6 +98,17 @@ export function ConfirmScreen({ state }: { state: OnboardingStateDto }) {
   const complete = useOnboardingComplete();
   const answer = useOnboardingAnswer();
   const [finishing, setFinishing] = useState(false);
+  /**
+   * v1.39 (Wave C, C2) — the ledger as the last write left it (M-j).
+   * `finish()` sends one PATCH per question the flow never showed; a failure
+   * midway used to leave a partly-passed ledger that the next attempt walked
+   * from the top again, because the `state` prop it read was the one this
+   * screen was rendered with. Every mutation answers the written state, so
+   * the ref carries the progress across attempts and
+   * `questionsToPassBeforeConfirm` — which lists pending steps only — then
+   * skips what the failed attempt already passed.
+   */
+  const ledger = useRef(state);
 
   const needs = state.needs;
   const forSomeoneElse = needs.recordTarget === "someone-else";
@@ -73,37 +124,39 @@ export function ConfirmScreen({ state }: { state: OnboardingStateDto }) {
    * after" just created: the route then applies the derivation to THAT
    * record and stamps this one complete without deriving, so the guardian's
    * own modules are never re-ordered around the child's answers.
+   *
+   * v1.39 (Wave C, C2) — this RETHROWS. `<BaselineForm>` owns the two
+   * buttons and releases them in its own `finally`, and it can only do that
+   * for a failure it is told about: the previous version caught, toasted and
+   * returned normally, which left both buttons disabled until a reload
+   * (research I10). The two managed-arm callers, which have no form around
+   * them, go through `finishReporting` below.
    */
   async function finish(managedRecordId?: string) {
     if (finishing) return;
     setFinishing(true);
     try {
-      // A question this flow never showed — Q6 for somebody with no
-      // unit-bearing area — is passed on the ledger before the completion,
-      // because the route derives only once EVERY question is answered or
-      // passed, and a step nobody was asked cannot be answered.
-      for (const id of questionsToPassBeforeConfirm(state)) {
-        await answer.mutateAsync({ step: id, status: "skipped" });
-      }
-      const { onboarding } = await complete.mutateAsync(
-        managedRecordId ? { managedRecordId } : undefined,
-      );
-      let written = onboarding ?? state;
-      if (!firstResultApplies(written)) {
-        // Nothing to offer, so the step is passed rather than left owed:
-        // a pending step would keep the setup reading as unfinished forever.
-        written = await answer.mutateAsync({
-          step: "first-result",
-          status: "skipped",
-        });
-      }
-      router.push(screenHref(nextScreen(written, "confirm") ?? "done"));
+      await runConfirmFinish({
+        ledger,
+        managedRecordId,
+        passStep: (step) => answer.mutateAsync({ step, status: "skipped" }),
+        complete: (id) =>
+          complete.mutateAsync(id ? { managedRecordId: id } : undefined),
+        navigate: (href) => router.push(href),
+      });
     } catch (err) {
-      toast.error(localizedApiError(err, t, "onboarding.errorGeneric"));
       setFinishing(false);
+      throw err;
     }
   }
 
+  /** The managed arm's two buttons: nothing above them reports a failure. */
+  function finishReporting(managedRecordId?: string) {
+    void finish(managedRecordId).catch((err) => {
+      toast.error(localizedApiError(err, t, "onboarding.errorGeneric"));
+    });
+  }
+
   const moduleNames = (keys: readonly (keyof typeof MODULE_REGISTRY)[]) =>
     keys.map((key) => t(MODULE_REGISTRY[key].labelKey)).join(", ");
 
@@ -196,7 +249,7 @@ export function ConfirmScreen({ state }: { state: OnboardingStateDto }) {
           </div>
           <ManagedProfileCreateForm
             submitLabel={t("onboarding.flow.confirm.managedCreate")}
-            onCreated={(profile) => void finish(profile.id)}
+            onCreated={(profile) => finishReporting(profile.id)}
           />
           <p className="text-sm">{t("onboarding.flow.confirm.managedLater")}</p>
           <div className="flex flex-wrap items-center justify-between gap-2 pt-2">
@@ -212,7 +265,7 @@ export function ConfirmScreen({ state }: { state: OnboardingStateDto }) {
             <Button
               type="button"
               variant="outline"
-              onClick={() => void finish()}
+              onClick={() => finishReporting()}
               disabled={finishing}
               className="min-h-11"
               data-slot="onboarding-finish-without-profile"