chore: merge executor worktree (worktree-agent-ab0c78658da0b8f33)

.planning/phases/12-initial-setup-wizard/12-05-SUMMARY.md

@@ -0,0 +1,88 @@
+---
+phase: 12-initial-setup-wizard
+plan: 05
+subsystem: setup-wizard-frontend
+tags: [setup, pwa, uat-gap-closure, a11y]
+requires:
+  - "GET /api/setup/status { setupComplete, dbName } (Plan 06)"
+  - "SetupStatusResponse.dbName?: string | null typed field (Plan 06)"
+provides:
+  - "Instance step intro copy trimmed (no DB-vs-env-file aside)"
+  - "Read-only, disabled DB-name field under App URL, populated from status dbName"
+  - "Instance field values lifted to SetupPage so Back navigation preserves them"
+affects:
+  - apps/pwa setup wizard Instance step (SetupPage.tsx)
+tech-stack:
+  added: []
+  patterns:
+    - "useQuery({ queryKey: ['setupStatus'], queryFn: fetchSetupStatus }) reads non-secret dbName into a read-only field"
+    - "Step-level field values lifted to the parent (SetupPage) so step unmount no longer drops entries"
+    - "Sensitive app password deliberately NOT lifted — stays in Step3 local state, cleared on unmount (T-12-15)"
+key-files:
+  created: []
+  modified:
+    - apps/pwa/src/routes/SetupPage.tsx
+    - apps/pwa/src/routes/SetupPage.test.tsx
+decisions:
+  - "D-12-05-LIFT: only the four non-secret Instance fields are lifted to SetupPage; the Fastmail app password is never lifted or persisted (T-12-15 preserved)."
+  - "D-12-05-DBNAME: DB-name field renders the dbName value only; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text, never as values (T-12-3DB)."
+  - "D-12-05-VALSTATE: Step2 validation state (db/oidc/vapid pass flags) is intentionally NOT lifted — only field values persist across Back; operator re-runs Save & Validate after returning."
+metrics:
+  duration_minutes: 9
+  completed: 2026-06-16
+---
+
+# Phase 12 Plan 05: Instance-Step Gap Closure (copy trim, DB-name field, Back persistence) Summary
+
+Closed UAT gaps 1, 3 (frontend half), and 4 on the PWA Instance step (`SetupPage.tsx`): dropped the confusing DB-vs-env-file implementation aside, added a read-only env-derived DB-name field so the "database connection verified" row has an on-screen referent, and lifted the four Instance field values into `SetupPage` so navigating Back from the Calendar step no longer wipes entered config.
+
+## What Was Built
+
+### Task 1 — Drop DB-vs-env aside + add read-only DB-name field (gaps 1, 3-frontend)
+Commit `35db5c5`.
+
+- **Gap 1**: Removed the sentence "These are written to the database — not your environment file." from the Instance step intro `<p>`, keeping the first sentence ("Enter your instance's connection details.").
+- **Gap 3 (frontend)**: Added a labelled, `readOnly` + `disabled` input ("Database") directly under the App URL field, populated from `fetchSetupStatus().dbName` via `useQuery({ queryKey: ['setupStatus'], staleTime: 0, retry: false })`. The field is greyed out (`--color-surface-dim` background, `--color-text-secondary` text), carries `aria-readonly="true"` and `tabIndex={-1}`, and shows `—` while loading/null. Helper text explains the DB is configured via the server's Docker environment (DB_HOST/DB_PORT/DB_USER/DB_PASSWORD as static names) and is not entered here. The existing "Database connection verified." ValidationRow is unchanged.
+- Tests assert the dropped sentence is absent, the DB field renders `readOnly`/`disabled`/`aria-readonly` with the mocked `dbName: 'familysync'`, and the existing DB validation row still appears on Save & Validate.
+
+### Task 2 — Preserve Instance fields across Back navigation (gap 4)
+Commit `a13fc11`.
+
+- Introduced an `InstanceFields` shape (`appUrl`, `oidcIssuer`, `oidcClientId`, `vapidPublicKey`) owned by `SetupPage` (`instanceFields` / `setInstanceFields`), passed to `Step2Config` as `fields` / `setFields` props. `Step2Config` now reads/writes these through the lifted setters instead of its own local `useState`. Validation/mutation logic is unchanged.
+- The Fastmail app password (Step 3) is **not** lifted — it remains in `Step3Credential` local state and is cleared on unmount when navigating away (T-12-15 preserved).
+- Tests: filling the Instance step, validating to GREEN, advancing to the Calendar step, then clicking Back restores all four Instance values; a second test confirms a typed app password is empty after Back→forward (Step 3 re-mounts fresh).
+
+## Verification
+
+- `cd apps/pwa && pnpm test -- SetupPage` → **263 passed (22 files)**.
+- `cd apps/pwa && pnpm typecheck` → clean (tsc + e2e tsconfig).
+- `grep -c "not your environment file" apps/pwa/src/routes/SetupPage.tsx` → **0**.
+- `grep -c "dangerouslySetInnerHTML" apps/pwa/src/routes/SetupPage.tsx` → **0**.
+- `grep -nE "sessionStorage|localStorage" apps/pwa/src/routes/SetupPage.tsx` → **no matches** (no client-side persistence of any field, secret or otherwise).
+- `grep -c "readOnly" apps/pwa/src/routes/SetupPage.tsx` → **1** (the DB-name field).
+- DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear only as static env-var names in helper/error copy — never fetched or rendered as values.
+
+## Deviations from Plan
+
+None — plan executed exactly as written. Implementation note: the two tasks both restructure the `Step2Config` signature/body and the same intro paragraph, so they were authored together and then committed as two atomic, individually-GREEN commits (Task 1 commit verified GREEN with 261 tests before Task 2's state-lifting and Back-navigation tests were added).
+
+## Threat Surface
+
+| Threat ID | Disposition | Outcome |
+|-----------|-------------|---------|
+| T-12-15 (app password disclosure) | mitigate | Preserved — password stays in Step3 local state, type="password", NOT lifted, NOT persisted to storage; cleared on unmount. Test asserts it is empty after Back→forward. |
+| T-12-14 (XSS in Instance/DB copy) | mitigate | All new copy + dbName rendered as plain-text JSX children; `dangerouslySetInnerHTML` grep = 0. |
+| T-12-3DB (DB secret/topology disclosure) | mitigate | Only `dbName` value is fetched and rendered; DB_HOST/DB_PORT/DB_USER/DB_PASSWORD appear solely as static env-var names in helper text. |
+
+No new security-relevant surface introduced beyond the planned `threat_model`.
+
+## Known Stubs
+
+None.
+
+## Self-Check: PASSED
+
+- `apps/pwa/src/routes/SetupPage.tsx` — modified, exists.
+- `apps/pwa/src/routes/SetupPage.test.tsx` — modified, exists.
+- Commit `35db5c5` (Task 1) — FOUND in git log.
+- Commit `a13fc11` (Task 2) — FOUND in git log.

apps/pwa/src/routes/SetupPage.test.tsx

@@ -210,9 +210,14 @@ describe('SetupPage — Already Locked screen', () => {
 describe('SetupPage — Step 2 VAPID validation (CR-01 gap)', () => {
   let queryClient: QueryClient;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     queryClient = makeQueryClient();
     vi.resetAllMocks();
+    const { fetchSetupStatus } = await import('../api/client.js');
+    (fetchSetupStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
+      setupComplete: false,
+      dbName: 'familysync',
+    });
   });
 
   /**
@@ -305,3 +310,141 @@ describe('SetupPage — Step 2 VAPID validation (CR-01 gap)', () => {
     });
   });
 });
+
+// ── Tests: Step 2 Instance copy + read-only DB-name field (gaps 1, 3-frontend) ──
+
+describe('SetupPage — Step 2 Instance copy + DB-name field (gaps 1, 3)', () => {
+  let queryClient: QueryClient;
+
+  beforeEach(async () => {
+    queryClient = makeQueryClient();
+    vi.resetAllMocks();
+    const { fetchSetupStatus } = await import('../api/client.js');
+    (fetchSetupStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
+      setupComplete: false,
+      dbName: 'familysync',
+    });
+  });
+
+  async function advanceToStep2() {
+    renderSetupPage(queryClient);
+    const continueBtn = await screen.findByRole('button', { name: 'Continue' });
+    fireEvent.click(continueBtn);
+    await screen.findByText('Instance Configuration');
+  }
+
+  it('Instance step intro no longer contains the DB-vs-env-file aside (gap 1)', async () => {
+    await advanceToStep2();
+    expect(screen.queryByText(/not your environment file/i)).toBeNull();
+    // First sentence is preserved
+    expect(screen.getByText(/Enter your instance/i)).toBeInTheDocument();
+  });
+
+  it('renders a read-only, disabled DB-name field populated from status dbName (gap 3)', async () => {
+    await advanceToStep2();
+    const dbField = await screen.findByLabelText('Database');
+    await waitFor(() => {
+      expect(dbField).toHaveValue('familysync');
+    });
+    expect(dbField).toHaveAttribute('readonly');
+    expect(dbField).toBeDisabled();
+    expect(dbField).toHaveAttribute('aria-readonly', 'true');
+  });
+
+  it('keeps the existing "Database connection verified" validation row available', async () => {
+    const { postSetupConfig, validateSetupDb, validateSetupOidc, validateSetupVapid } =
+      await import('../api/client.js');
+    (postSetupConfig as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupDb as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupOidc as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupVapid as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+
+    await advanceToStep2();
+    fireEvent.change(screen.getByLabelText('App URL'), { target: { value: 'https://app.example.com' } });
+    fireEvent.change(screen.getByLabelText('OIDC issuer URL'), { target: { value: 'https://auth.example.com' } });
+    fireEvent.change(screen.getByLabelText('OIDC client ID'), { target: { value: 'familysync' } });
+    fireEvent.change(screen.getByLabelText('VAPID public key'), { target: { value: 'BHtest123' } });
+    fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' }));
+
+    await waitFor(() => {
+      expect(screen.getByText('Database connection verified.')).toBeInTheDocument();
+    });
+  });
+});
+
+// ── Tests: Back navigation preserves Instance fields (gap 4) ──────────────────
+
+describe('SetupPage — Back navigation preserves Instance fields (gap 4)', () => {
+  let queryClient: QueryClient;
+
+  beforeEach(async () => {
+    queryClient = makeQueryClient();
+    vi.resetAllMocks();
+    const { fetchSetupStatus } = await import('../api/client.js');
+    (fetchSetupStatus as ReturnType<typeof vi.fn>).mockResolvedValue({
+      setupComplete: false,
+      dbName: 'familysync',
+    });
+  });
+
+  /**
+   * Helper: fill the four Instance fields, run validation to GREEN, advance to
+   * the Calendar step (step 3).
+   */
+  async function fillStep2AndAdvance() {
+    const { postSetupConfig, validateSetupDb, validateSetupOidc, validateSetupVapid } =
+      await import('../api/client.js');
+    (postSetupConfig as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupDb as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupOidc as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+    (validateSetupVapid as ReturnType<typeof vi.fn>).mockResolvedValue(undefined);
+
+    renderSetupPage(queryClient);
+    fireEvent.click(await screen.findByRole('button', { name: 'Continue' }));
+    await screen.findByText('Instance Configuration');
+
+    fireEvent.change(screen.getByLabelText('App URL'), { target: { value: 'https://app.example.com' } });
+    fireEvent.change(screen.getByLabelText('OIDC issuer URL'), { target: { value: 'https://auth.example.com' } });
+    fireEvent.change(screen.getByLabelText('OIDC client ID'), { target: { value: 'familysync' } });
+    fireEvent.change(screen.getByLabelText('VAPID public key'), { target: { value: 'BHtest123' } });
+    fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' }));
+
+    // After all validations pass, the Continue button appears
+    fireEvent.click(await screen.findByRole('button', { name: 'Continue' }));
+    await screen.findByText('Fastmail Credential');
+  }
+
+  it('restores all four Instance field values after navigating Back from Calendar step', async () => {
+    await fillStep2AndAdvance();
+
+    // Now on step 3 (Calendar). Navigate Back.
+    fireEvent.click(screen.getByRole('button', { name: 'Back' }));
+    await screen.findByText('Instance Configuration');
+
+    expect(screen.getByLabelText('App URL')).toHaveValue('https://app.example.com');
+    expect(screen.getByLabelText('OIDC issuer URL')).toHaveValue('https://auth.example.com');
+    expect(screen.getByLabelText('OIDC client ID')).toHaveValue('familysync');
+    expect(screen.getByLabelText('VAPID public key')).toHaveValue('BHtest123');
+  });
+
+  it('does NOT persist the Fastmail app password across Back/forward navigation (T-12-15)', async () => {
+    await fillStep2AndAdvance();
+
+    // On step 3: type a password into the app password field.
+    const pwField = screen.getByLabelText('App password');
+    fireEvent.change(pwField, { target: { value: 'super-secret-pw' } });
+    expect(pwField).toHaveValue('super-secret-pw');
+
+    // Back to step 2 — field values are preserved, but validation state is not
+    // lifted, so re-run Save & Validate to surface Continue, then advance to step 3.
+    fireEvent.click(screen.getByRole('button', { name: 'Back' }));
+    await screen.findByText('Instance Configuration');
+    // Fields are still populated (gap 4), so just re-validate.
+    fireEvent.click(screen.getByRole('button', { name: 'Save & Validate' }));
+    fireEvent.click(await screen.findByRole('button', { name: 'Continue' }));
+    await screen.findByText('Fastmail Credential');
+
+    // Step 3 re-mounted with fresh local state — the app password is NOT persisted.
+    expect(screen.getByLabelText('App password')).toHaveValue('');
+  });
+});

apps/pwa/src/routes/SetupPage.tsx

@@ -21,9 +21,10 @@
  */
 
 import { useState, useRef, useEffect } from 'react';
-import { useMutation } from '@tanstack/react-query';
+import { useMutation, useQuery } from '@tanstack/react-query';
 import { ShieldCheck, CheckCircle, AlertCircle, Loader2 } from 'lucide-react';
 import {
+  fetchSetupStatus,
   postSetupConfig,
   validateSetupDb,
   validateSetupOidc,
@@ -429,19 +430,41 @@ function Step1Welcome({ onContinue, stepHeadingRef }: Step1Props) {
 
 // ── Step 2: Instance Configuration ───────────────────────────────────────────
 
+interface InstanceFields {
+  appUrl: string;
+  oidcIssuer: string;
+  oidcClientId: string;
+  vapidPublicKey: string;
+}
+
 interface Step2Props {
   onBack: () => void;
   onSuccess: () => void;
   stepHeadingRef: React.RefObject<HTMLHeadingElement | null>;
+  /** Lifted to SetupPage so values survive step unmount (gap 4: Back preserves entries). */
+  fields: InstanceFields;
+  setFields: React.Dispatch<React.SetStateAction<InstanceFields>>;
 }
 
-function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) {
+function Step2Config({ onBack, onSuccess, stepHeadingRef, fields, setFields }: Step2Props) {
-  const [appUrl, setAppUrl] = useState('');
+  const { appUrl, oidcIssuer, oidcClientId, vapidPublicKey } = fields;
-  const [oidcIssuer, setOidcIssuer] = useState('');
+  const setAppUrl = (v: string) => setFields((f) => ({ ...f, appUrl: v }));
-  const [oidcClientId, setOidcClientId] = useState('');
+  const setOidcIssuer = (v: string) => setFields((f) => ({ ...f, oidcIssuer: v }));
-  const [vapidPublicKey, setVapidPublicKey] = useState('');
+  const setOidcClientId = (v: string) => setFields((f) => ({ ...f, oidcClientId: v }));
+  const setVapidPublicKey = (v: string) => setFields((f) => ({ ...f, vapidPublicKey: v }));
   const [fieldError, setFieldError] = useState<string | null>(null);
 
+  // Gap 3 (frontend): fetch the env-derived, non-secret DB name so the
+  // "database connection verified" row below has an on-screen referent.
+  // Only dbName is surfaced — DB_HOST/DB_USER/DB_PASSWORD are never fetched (T-12-3DB).
+  const { data: setupStatus } = useQuery({
+    queryKey: ['setupStatus'],
+    queryFn: fetchSetupStatus,
+    staleTime: 0,
+    retry: false,
+  });
+  const dbName = setupStatus?.dbName ?? '';
+
   const [validationRows, setValidationRows] = useState<Pick<ValidationRowStatus, 'db' | 'oidc' | 'vapid'>>({
     db: 'idle',
     oidc: 'idle',
@@ -553,8 +576,7 @@ function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) {
           lineHeight: 1.5,
         }}
       >
-        Enter your instance&apos;s connection details. These are written to the database — not
+        Enter your instance&apos;s connection details.
-        your environment file.
       </p>
 
       {/* App URL */}
@@ -574,6 +596,32 @@ function Step2Config({ onBack, onSuccess, stepHeadingRef }: Step2Props) {
         <div style={helperStyle}>The public URL where FamilySync is reachable.</div>
       </div>
 
+      {/* Database (read-only, env-derived) — gives the DB validation row below a referent */}
+      <div style={{ marginBottom: 'var(--space-4, 16px)' }}>
+        <label htmlFor="setup-db-name" style={labelStyle}>
+          Database
+        </label>
+        <input
+          id="setup-db-name"
+          type="text"
+          value={dbName || '—'}
+          readOnly
+          disabled
+          aria-readonly="true"
+          tabIndex={-1}
+          style={{
+            ...inputStyle(false),
+            background: 'var(--color-surface-dim, #f7f7f8)',
+            color: 'var(--color-text-secondary, #6b7280)',
+            cursor: 'default',
+          }}
+        />
+        <div style={helperStyle}>
+          Configured via the server&apos;s Docker environment (<code>DB_HOST</code>,{' '}
+          <code>DB_PORT</code>, <code>DB_USER</code>, <code>DB_PASSWORD</code>) — not entered here.
+        </div>
+      </div>
+
       {/* OIDC Issuer */}
       <div style={{ marginBottom: 'var(--space-4, 16px)' }}>
         <label htmlFor="setup-oidc-issuer" style={labelStyle}>
@@ -1035,6 +1083,16 @@ export function SetupPage({ alreadyLocked = false }: SetupPageProps) {
   const [terminal, setTerminal] = useState<TerminalState>(alreadyLocked ? 'locked' : null);
   const stepHeadingRef = useRef<HTMLHeadingElement>(null);
 
+  // Gap 4: Instance-step field values are lifted here so they survive Step2 unmount.
+  // The Fastmail app password (Step 3) is deliberately NOT lifted — it stays in
+  // Step3Credential local state and is cleared on unmount (T-12-15 preserved).
+  const [instanceFields, setInstanceFields] = useState<InstanceFields>({
+    appUrl: '',
+    oidcIssuer: '',
+    oidcClientId: '',
+    vapidPublicKey: '',
+  });
+
   // Focus the step heading on step change for a11y (D-04 focus management)
   useEffect(() => {
     if (stepHeadingRef.current) {
@@ -1129,6 +1187,8 @@ export function SetupPage({ alreadyLocked = false }: SetupPageProps) {
             onBack={() => setStep(1)}
             onSuccess={() => setStep(3)}
             stepHeadingRef={stepHeadingRef}
+            fields={instanceFields}
+            setFields={setInstanceFields}
           />
         )}
         {step === 3 && (