Setup: per-section collapsible "Advanced" for sane-default fields #168

Closed
opened 2026-09-06 00:02:28 +00:00 by crueber · 3 comments
Owner

Setup: per-section collapsible "Advanced" for sane-default fields

The /setup page shows every field flat. Fields with sane defaults that most operators never touch should live in a per-section "Advanced" subsection, hidden by default (collapsed <details>-style or equivalent) inside the section they belong to.

Sections called out (audit each; default-hidden advanced for the sane-default knobs, essential fields stay visible): cache, server concurrency + timeouts, wal, maintenance, compaction, bundles, git, telemetry, import. Evaluate the rest (auth, store backends, SSH, notifications, checks, releases, etc.) with the same lens: essential-to-first-boot stays visible; tunable-with-sane-default goes advanced.

Suggested shape (follow existing conventions)

  • FIELDS entries gain an advanced: true flag (same pattern as modes/backends); Setup.jsx renders per-section advanced groups collapsed by default.
  • Hidden fields still validate/save exactly as today (no behavior change to the overrides channel, test endpoint, or save path); expanded state is UI-only.
  • Keyboard accessible (native disclosure or aria-expanded), dark + light.

Acceptance criteria

  • Named sections above have collapsed Advanced groups; essential fields remain visible.
  • All other sections evaluated (list the verdict per section in the PR).
  • Save/test/validate behavior unchanged (existing setup tests green + new tests for grouping).
  • node --test green; browser check of /setup (expand/collapse, save round-trip) both themes, zero console errors; no new deps.
# Setup: per-section collapsible "Advanced" for sane-default fields The `/setup` page shows every field flat. Fields with sane defaults that most operators never touch should live in a per-section **"Advanced" subsection, hidden by default** (collapsed `<details>`-style or equivalent) inside the section they belong to. Sections called out (audit each; default-hidden advanced for the sane-default knobs, essential fields stay visible): **cache**, **server concurrency + timeouts**, **wal**, **maintenance**, **compaction**, **bundles**, **git**, **telemetry**, **import**. Evaluate the rest (auth, store backends, SSH, notifications, checks, releases, etc.) with the same lens: essential-to-first-boot stays visible; tunable-with-sane-default goes advanced. ## Suggested shape (follow existing conventions) - `FIELDS` entries gain an `advanced: true` flag (same pattern as `modes`/`backends`); Setup.jsx renders per-section advanced groups collapsed by default. - Hidden fields still validate/save exactly as today (no behavior change to the overrides channel, test endpoint, or save path); expanded state is UI-only. - Keyboard accessible (native disclosure or aria-expanded), dark + light. ## Acceptance criteria - [ ] Named sections above have collapsed Advanced groups; essential fields remain visible. - [ ] All other sections evaluated (list the verdict per section in the PR). - [ ] Save/test/validate behavior unchanged (existing setup tests green + new tests for grouping). - [ ] `node --test` green; browser check of /setup (expand/collapse, save round-trip) both themes, zero console errors; no new deps.
Author
Owner

Fixed by PR #169 (branch fix/issue-168): per-section collapsed Advanced groups on /setup, save/test/validate byte-identical, 345/345 node tests green. Browser check (expand/collapse + save round-trip, both themes, zero console errors) still needed before merge — no runnable browser daemon in this environment.

Fixed by PR #169 (branch fix/issue-168): per-section collapsed Advanced groups on /setup, save/test/validate byte-identical, 345/345 node tests green. Browser check (expand/collapse + save round-trip, both themes, zero console errors) still needed before merge — no runnable browser daemon in this environment.
Author
Owner

Review: PR #169 (fix/issue-168) — setup per-section collapsible Advanced

Verified in scratch worktree at 5941083 (removed afterward); main worktree read-only (fetch+diff only). No fixes needed — nothing pushed.

Checklist (all pass)

  • Advanced flags sane, essentials visible. All server.auth.* rows carry no advanced flag (web/src/lib/setup.js:213-228) — session_secret, oauth_client_secret, tokens, allowlists all stay open. store.s3.access_key_env/secret_key_env (:243-245), events.webhook_secret (:343), import.url_allowlist (:347) also unflagged. auth and placement sections are fully unflagged (pinned by test). Deliberate advanced: true on wal.push_broker_token (:271), upstream.token_env (:324), import.allow_private_networks/allow_file_urls (:348-349) is fine: empty-by-default opt-ins, fail-closed defaults preserved, first boot needs none of them.
  • Unknown-keys-default-essential verified: splitAdvanced (setup.js:182-189) routes unknown keys to essential; covered by setup-advanced.test.js "treats unknown keys as essential".
  • Collapsed != unmounted, save path unfiltered: filterSetupToVisible (setup.js:161-170) gates only on backends, knows nothing about advanced; both essential and advanced rows render through the same renderRow inside the always-mounted <details> (Setup.jsx:334-395). Live-verified: [data-key="server.http2"] present in DOM while its group is closed.
  • Error badge on collapsed summary: advErrorCount (Setup.jsx:394) surfaces hidden-row hint count on the <summary> via .chip-draft. Live-verified: poisoning collapsed cache.prewarm_parallelism=0 shows "Advanced / 1 issue" on the closed group, and Validate reports cache.prewarm_parallelism: must be >= 1, got 0.
  • Disclosure only when applicable: advVisibleCount (Setup.jsx:390-393) combines advanced with the mode/backend gates; auth/placement render no group (browser-confirmed).
  • Keyboard: native <details>/<summary> (Setup.jsx:414-428), summary focusable (tabIndex >= 0 in browser). No JS open-state.
  • Per-section verdicts plausible: 13 grouped sections each keep >=1 essential and >=1 advanced row; auth/placement all-visible with stated rationale (mode-gated/required-or-sensitive; globs have no sane default). Browser shows exactly these 13 groups; releases/attachments cards correctly get none.
  • Save-payload equivalence tested: unit test pins normalizeSetup identical expanded vs collapsed plus every advanced key present in overrides; browser Validate round-trip (poisoned -> client error; fixed -> 200 {errors: []}) confirms end to end.
  • No new deps: diff is 5 files only (setup.js, Setup.jsx, ui.css, setup-advanced.test.js, 12_web_ui.md). package.json untouched.
  • Dark + light: .setup-advanced / .setup-advanced-toggle carry dark: variants (ui.css:80-83); error chip reuses .chip-draft which already has dark: (ui.css:61-62). Browser: 13 groups, distinct toggle colors in both themes (light lab(47.9…) vs dark lab(65.6…)), zero console/page errors in both.
  • Doc entry accurate: 12_web_ui.md rendering bullet + FIXED (issue #168) appendix match the code (mounted-not-hidden, gates, badge, no-group cases, headless cover named).

Test results

  • node --test web/test/unit/*.test.js in scratch worktree: 345/345 pass (incl. new setup-advanced.test.js 20/20, setup-form.test.js 46/46). Note: 3 unrelated suites (data-guard, reaction-cache, tolerate-missing) fail without web/node_modules (missing solid-js import); with the main worktree's node_modules symlinked read-only, the full suite is green. Pre-existing env quirk, not this PR.
  • vite build: clean (121 modules, 1.73s).
  • Real browser (hub CDP daemon :9222, PR binary on scratch data dir, canonical host walgit.localhost:18099/setup): 13 collapsed-by-default native groups; expand/collapse toggles; collapsed row in DOM; error badge count; Validate blocked-then-200 flow; theme toggle both directions; no console or page errors.

MERGE RECOMMENDATION: ready to merge

No findings requiring changes. Not merging per instructions.

# Review: PR #169 (fix/issue-168) — setup per-section collapsible Advanced Verified in scratch worktree at `5941083` (removed afterward); main worktree read-only (`fetch`+`diff` only). No fixes needed — nothing pushed. ## Checklist (all pass) - **Advanced flags sane, essentials visible.** All `server.auth.*` rows carry no `advanced` flag (`web/src/lib/setup.js:213-228`) — session_secret, oauth_client_secret, tokens, allowlists all stay open. `store.s3.access_key_env/secret_key_env` (:243-245), `events.webhook_secret` (:343), `import.url_allowlist` (:347) also unflagged. `auth` and `placement` sections are fully unflagged (pinned by test). Deliberate `advanced: true` on `wal.push_broker_token` (:271), `upstream.token_env` (:324), `import.allow_private_networks/allow_file_urls` (:348-349) is fine: empty-by-default opt-ins, fail-closed defaults preserved, first boot needs none of them. - **Unknown-keys-default-essential verified:** `splitAdvanced` (`setup.js:182-189`) routes unknown keys to `essential`; covered by `setup-advanced.test.js` "treats unknown keys as essential". - **Collapsed != unmounted, save path unfiltered:** `filterSetupToVisible` (`setup.js:161-170`) gates only on `backends`, knows nothing about `advanced`; both essential and advanced rows render through the same `renderRow` inside the always-mounted `<details>` (`Setup.jsx:334-395`). Live-verified: `[data-key="server.http2"]` present in DOM while its group is closed. - **Error badge on collapsed summary:** `advErrorCount` (`Setup.jsx:394`) surfaces hidden-row hint count on the `<summary>` via `.chip-draft`. Live-verified: poisoning collapsed `cache.prewarm_parallelism=0` shows "Advanced / 1 issue" on the closed group, and Validate reports `cache.prewarm_parallelism: must be >= 1, got 0`. - **Disclosure only when applicable:** `advVisibleCount` (`Setup.jsx:390-393`) combines `advanced` with the mode/backend gates; `auth`/`placement` render no group (browser-confirmed). - **Keyboard:** native `<details>/<summary>` (`Setup.jsx:414-428`), summary focusable (`tabIndex >= 0` in browser). No JS open-state. - **Per-section verdicts plausible:** 13 grouped sections each keep >=1 essential and >=1 advanced row; `auth`/`placement` all-visible with stated rationale (mode-gated/required-or-sensitive; globs have no sane default). Browser shows exactly these 13 groups; `releases`/`attachments` cards correctly get none. - **Save-payload equivalence tested:** unit test pins `normalizeSetup` identical expanded vs collapsed plus every advanced key present in overrides; browser Validate round-trip (poisoned -> client error; fixed -> `200 {errors: []}`) confirms end to end. - **No new deps:** diff is 5 files only (setup.js, Setup.jsx, ui.css, setup-advanced.test.js, 12_web_ui.md). `package.json` untouched. - **Dark + light:** `.setup-advanced` / `.setup-advanced-toggle` carry `dark:` variants (`ui.css:80-83`); error chip reuses `.chip-draft` which already has `dark:` (`ui.css:61-62`). Browser: 13 groups, distinct toggle colors in both themes (light `lab(47.9…)` vs dark `lab(65.6…)`), zero console/page errors in both. - **Doc entry accurate:** `12_web_ui.md` rendering bullet + `FIXED (issue #168)` appendix match the code (mounted-not-hidden, gates, badge, no-group cases, headless cover named). ## Test results - `node --test web/test/unit/*.test.js` in scratch worktree: **345/345 pass** (incl. new `setup-advanced.test.js` 20/20, `setup-form.test.js` 46/46). Note: 3 unrelated suites (`data-guard`, `reaction-cache`, `tolerate-missing`) fail without `web/node_modules` (missing `solid-js` import); with the main worktree's `node_modules` symlinked read-only, the full suite is green. Pre-existing env quirk, not this PR. - `vite build`: clean (121 modules, 1.73s). - Real browser (hub CDP daemon :9222, PR binary on scratch data dir, canonical host `walgit.localhost:18099/setup`): 13 collapsed-by-default native groups; expand/collapse toggles; collapsed row in DOM; error badge count; Validate blocked-then-200 flow; theme toggle both directions; **no console or page errors**. ## MERGE RECOMMENDATION: ready to merge No findings requiring changes. Not merging per instructions.
Author
Owner

Fixed by PR #169 (review clean incl. real-browser pass; 345/345 node tests), merged. Closing.

Fixed by PR #169 (review clean incl. real-browser pass; 345/345 node tests), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:27:16 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#168
No description provided.