[mchestr]

Why

The admin area had grown to 17 sidebar items under four group headings, a hamburger sheet plus a tab bar on phones, and one /admin/settings page with 16 sections in six groups. Finding a feature's controls meant knowing whether they lived on the feature's page or somewhere on Settings. This PR implements the restructure from the usability audit: ten flat items, each feature's controls on its own page as a Settings tab, and /admin/settings kept for what is global.

What changes

Sidebar — ten items, no group headings

Item	Tabs
Overview
Assistant
Members	Users · Invites · Affiliates
Announcements
Rewind	Season · Playground · Shares · Settings
Discord	Activity · Settings
AI	Activity · Cost · Settings
Automations	Watchlist sync · Inactive members · Request limits · Queues
Audit log
Settings	Connections · Billing · System

• lib/admin/nav.ts stays the one list: items carry children (the tab strip) and also (sibling paths that light the item). The sidebar, the mobile tab bar, page titles, the command menu and each section's tab strip read it. CLAUDE.md gains an "Admin navigation" subsection with the rules: a feature's controls live on the feature's page; the sidebar budget is ten items; settingsSection(id) is the only way to link to a settings section.
• URLs are unchanged. Sibling routes keep their paths through a route group per section (app/admin/(members)/, (rewind)/, (ai)/, (automations)/, and the discord/ segment). Each group's layout.tsx is export default sectionLayout("<id>"), which draws SectionTabs from the item's permitted children; detail routes (/admin/users/[userId], …) render no strip. Layouts are not authorisation boundaries — every page keeps its own assertAdmin()/permission check.
• Four new pages host the sections that used to sit on Settings, with the same components, LastSaved footers and test ids: /admin/rewind/settings, /admin/discord/settings (incl. channel stats), /admin/ai/settings (providers, caps, judge, assistant sources) and /admin/request-limits. The actions that save those sections revalidate the new pages. Old /admin/settings#<moved id> links redirect to the section's new page.
• /admin/settings keeps Connections (Plex, Tautulli, Overseerr, other), Billing (Stripe) and System (time zone, API access, privacy, system). "Test all connections" counts only the sections the page shows.
• Phones: one menu. The hamburger sheet is gone; the tab bar is Overview · Members · Rewind · Automations · More (capped at four link tabs; More lists every item). The failed-jobs dot sits on the tab that carries the badge (Automations) instead of on More. A section strip wider than the screen scrolls its selected tab into view.
• Command menu lists "Page › Tab" entries, so "request" finds Automations › Request limits. The nav provider keeps the per-role href and permitted children the server computed, so a Support user's folded item opens its first permitted tab.
• Support role sees Assistant, Members (Users and Affiliates tabs, no Invites) and Audit log; the refused-paths E2E list gains the four new admin-only pages.
• Every section tab has a loading.tsx, so switching tabs shows the skeleton instead of a frozen strip.
• Along the way: LastSaved … by <admin> links to the audit event; the unused "Sound" flag row leaves the Season page; a shared useLeaveGuard keeps the reload/close prompt for sections that no longer sit inside the Settings layout (and gives it to settings sheets, which never had it); the time-zone note inside the open Rewind form no longer offers a link that would drop the draft; settingsSection() ignores keys inherited from Object; the Watchlist page gets "Sync now"; dead icons and the unread invites count are removed; the Members badge reads "N to review" for pending applications; docs, setup-wizard and assistant copy name the new locations.

Follow-ups (not in this PR)

• Admin settings: guard client-side navigation away from a dirty section #515 — Client-side navigation away from a dirty stand-alone section (switching tabs or sidebar items) drops the draft without a prompt; beforeunload covers reload and close only. The old one-page Settings kept drafts while moving between sections.
• Command menu: match multi-word queries and old page names #516 — The command menu matches one substring on the label ("automations request" finds nothing; old names such as "cost analysis" or "job queues" no longer match).
• Admin nav: keep a folded item lit when a role may open only a later, nested tab #517 — navFor rewrites a gated item's href to the first permitted tab; a role that may open only a later, nested tab would not light the item (no such role exists today; a nav test fails the moment one does).
• Admin: two pre-existing dead ends surfaced by the nav review #518 — Pre-existing: Support's read-only user list links names to a page that refuses Support; the Overview Services tile sends Sonarr/Radarr/Jellyfin to anchors no element carries.
• Visual baselines will drift (sidebar, phone chrome, Settings nav); the advisory Visual Diffs jobs will show it and the Visual Baselines workflow refreshes them.

Notes for review

• .claude/skills/rewind-design-system/SKILL.md changes one path token (app/admin/users/… → app/admin/(members)/users/…) because scripts/__tests__/project-skills.test.ts checks that named paths exist. CLAUDE.md asks for a /security-review of any .claude/skills change: two reviews inspected it (prose only; no instructions, shell, links or frontmatter), but no separate /security-review has run — that is yours to run or waive before merge.
• sectionLayout() returns the async server layout component; typecheck (incl. next typegen) passes and CI's Build job exercises next build.
• main was merged in after the skill-sources work (Assistant skills, phase 1 #513) landed: the new block renders on the AI Settings tab through the shared loader; its action, E2E spec, executor copy and docs now name /admin/ai/settings.
• Checks on the final head: typecheck, lint (0 errors), design-system --strict, knip, env:docs --check, mcp:docs --check; full jest — the only failures (get-share-safe "is null after the season closes", retention fallback "runs once per 24 hours") fail identically on main. Playwright runs in CI.

How it was built

Planned from the usability audit, then executed as thirteen tasks on separate worktrees by implementer agents with a task-scoped review each (spec compliance + code quality), fix rounds where a review found an Important issue, merges as the union of both sides, a whole-branch review at the end and one fix wave with a scoped re-review. Per-task commits are kept so pieces can be cherry-picked or reverted individually.

https://claude.ai/code/session_011TLtULGdZXTe6L61jNKqRF

---

Generated by Claude Code

[claude]

Claude finished @mchestr's task in 42s —— View job

---

Review of #514: Admin navigation, ten items with section tabs

Verdict: no blocking issues found. I read the code only. I did not run tests or the build, so CI is the check for those.

What I checked

• lib/admin/nav.ts: isActive, navFor and childrenOf are consistent with the documented behaviour. navFor rewrites an item's href to the first permitted tab, which is what the Support role needs. Admin nav: keep a folded item lit when a role may open only a later, nested tab #517 already tracks the nested-tab edge case.
• sectionLayout: it throws on an unknown id when called at module load, so a renamed item fails early. The layout only draws tabs. The module and the PR description both say it is not an authorisation boundary.
• New pages (rewind/ai/discord settings, request-limits): each calls assertAdmin() first, then connection(), then loads data. This matches the CLAUDE.md rule that every page keeps its own check.
• Revalidation: the moved sections' actions now target the new paths: request tiers, Discord, the setup Discord save, Rewind settings, AI providers and judge settings. The remaining /admin/settings revalidations (Stripe, API tokens, servers) are for sections that stayed on Settings.
• app/admin/settings/page.tsx: the old data loads are removed, so the page no longer fetches data it doesn't show. otherStatus now uses SECTION_HEALTH_KEYS, which removes the duplicated key list.

Non-blocking notes

1. Health on the Settings page after moved-section saves. getSettingsHealth() is cached by tag. The Discord and AI save paths expire the tag (expireTag(CONNECTION_HEALTH_TAG) in actions/discord.ts). I did not confirm that every moved-section save that changes health does the same. Worth a quick look at the Rewind and AI settings actions, since those pages and the Overview tile now read health. This is probably fine given the per-request rendering you described.
2. /security-review for .claude/skills/rewind-design-system/SKILL.md. CLAUDE.md requires one for any change under .claude/skills/**. The change is a single path token, so waiving it is reasonable, but it is your call as the description says.
3. Dirty-draft loss on tab switch (Admin settings: guard client-side navigation away from a dirty section #515). This is a UX regression from the old one-page Settings, and it is tracked. Consider whether the inline editors in SettingsSection could warn on in-app navigation before the next release, since users now move between tabs more often.
4. Unit Tests failure. The share-safe.property.test.ts failure is unrelated to this diff, and Random share-safe titles never contain one another (flaky property test) #519 is the fix. Merge after that lands and the job re-runs green.

Test coverage looks thorough: section layouts, tab loading states for every tab, the command menu, the "every role lights every tab" nav test, and per-page tests for the four new pages.
· branch feat/admin-nav-restructure

[mchestr]

Checked the six points against the code:

1. Revalidation readers. getSettingsAuditMeta() is read by the five settings-hosting pages, each rendering only its own sections' LastSaved, so a save revalidates the one page that shows it. getRequestTierSettings() is also read by lib/data/community-stats.ts (Discord settings) and the MCP read tool; getSettingsHealth() by the Settings, Rewind-settings and AI-settings pages and the Overview tile. Those pages are uncached and render per request, and Next 16's client router keeps dynamic pages at staleTimes.dynamic = 0, so navigating to them after a save shows fresh data without an extra revalidatePath. Nothing is left stale.
2. sectionLayout fail-fast. components/admin/__tests__/section-layout.test.tsx and app/admin/__tests__/tab-loading.test.ts both iterate every children-bearing ADMIN_NAV item, so a renamed id fails in jest before next build.
3. Dirty-draft navigation. Tracked as Admin settings: guard client-side navigation away from a dirty section #515, linked from the description.
4. First permitted tab vs. isActive. Added every role lights every tab it may open to lib/admin/__tests__/nav.test.ts (runs over ROLE_KEYS); the latent nested-tab case is Admin nav: keep a folded item lit when a role may open only a later, nested tab #517.
5. Agreed; tab-loading.test.ts guards it.
6. /security-review has not run here; the description now says so plainly for @mchestr to run or waive.

Nits: the nav.ts module JSDoc is fixed in the same commit; "Test all connections" already has exact-count tests in components/admin/settings/__tests__/settings-page-actions.test.tsx (success, one failure through the Tautulli→Rewind dependency, five failures, non-failure statuses).

---

Generated by Claude Code

[mchestr]

Unit Tests is red on d1cb98e for a reason outside this PR. The failing suite is lib/wrapped/data/__tests__/share-safe.property.test.ts ("never carries a hidden title"), a fast-check property over random Rewind rows in lib/wrapped/data, which this PR does not touch. The shrunk counterexample (seed -1167034155) draws a first title ZqtoLocaa and a last title ZqtoLoca; with only the last hidden, the test's substring check matches the visible first title and reports a leak that is not one. It reproduces locally with that seed on main too, so it is a seed-dependent weakness of the generator, not a regression here. The other 961 suites pass.

Fix: #519 makes the generator reject title sets where one title contains another (reproduced with the seed, passes with it, soaked for 2,000 runs). I'll re-run the Unit Tests job once the rest of the workflow finishes; a different seed should pass.

---

Generated by Claude Code

[mchestr]

E2E Tests was red for a reason that was this PR's, now fixed in d8f1727. Ten e2e/landmarks.spec.ts cases (every admin route with a section tab strip) failed at "first Tab focuses the skip link": the strip called scrollIntoView on its selected tab on mount, and in Chromium that also moves the sequential-focus starting point, so the first Tab landed past the skip link. Reproduced the browser behaviour in isolation with Playwright (a visible element that is scrollIntoView'd steals the Tab start), then changed the strip to move its list's own scrollLeft by the selected tab's overflow instead — same effect on phones, no focus side effect. Unit tests cover the three geometries and pin that scrollIntoView is never called.

The other failure in that run was the unrelated share-safe property seed (see above, fix in #519); the only flaky case (analytics.spec.ts "page 2 of AI activity keeps the member filter") passed on retry. Everything else — Build, Integration, Schema Drift, Rewind E2E, Rewind Performance, Image Smoke Test, Visual Diffs — was green.

---

Generated by Claude Code

[mchestr]

On note 1 (health cache after moved-section saves): getConnectionHealth() is the 5-minute unstable_cache of the service probes plus the ai entry (llmDisabled, active-provider count); discord, stripe and rewind health are read live on every call, so no Discord or Rewind settings save can leave them stale. The saves that change the cached part expire CONNECTION_HEALTH_TAG: server saves (admin-servers.ts), the AI master switch (setLLMDisabled) and the Discord token (discord.ts). The one that does not is saving an LLM provider (admin-llm-providers.ts), so the AI entry can read "not configured" for up to five minutes after the first provider is added — the same as on main, where that action revalidated /admin/settings without expiring the tag either. A one-line expireTag(CONNECTION_HEALTH_TAG) there is the fix; I'll fold it into the next code push on this branch if one is needed, otherwise it goes with #518 as a follow-up rather than restarting CI for it now.

---

Generated by Claude Code