feat(site): implement AI settings page with providers management UI#25551
feat(site): implement AI settings page with providers management UI#25551tracyjohnsonux wants to merge 13 commits into
Conversation
Rebases the frontend changes from #25328 onto current main. All three dependency PRs (#24892, #24893, #24894) are already merged, so this lands the UI layer cleanly. - AISettingsLayout with sidebar, mounted under /ai/settings - ProvidersPage list view + table of configured providers - AddProviderPage and UpdateProviderPage with shared ProviderForm - ProviderIcon and ProviderRow components with Storybook stories - aiProviders query + CRUD mutation hooks - viewAnyAIProvider permission against ai_provider RBAC resource - Navbar deployment dropdown entry for AI settings
- Create AISettingsSidebarView with AI Governance, Providers, and Manage Coder Agents entries - Move AI routes to /ai/settings with dedicated AISettingsLayout - Add AI entry to admin settings dropdown in navbar - Remove AI items from deployment sidebar - Switch provider pages to SettingsHeader for consistent alignment - Streamline ProviderForm layout with side-by-side fields - Move enable toggle from form to update page header - Add model count display and disabled badge to update page
Add -ml-3 to offset the subtle button's px-3 padding so the arrow icon sits flush with the text content below it.
Add validateOnMount so isValid reflects required-field state from the start. Button stays disabled until the user has both changed something and all required fields pass validation.
Remove provider-specific model counting logic and link to /agents/settings/models instead.
Point the Providers entry in the agents settings sidebar to the new AI settings page and add an ArrowUpRightIcon to indicate the external navigation.
Port from PR #25328: warn before leaving a dirty provider form via useBlocker (in-app) and beforeunload (tab close). Parents switch to mutateAsync so the submitting state stays true through navigation. Also fixes the dirty-reset after save (null vs undefined check).
The SettingsPanel.tsx change (Providers → /ai/settings link) moved to PR #25574 for separate review.
Brings Tracey's UI improvements from #25551 onto this branch: - New top-level AI section at /ai/settings with its own layout and sidebar (AISettingsLayout, AISettingsSidebarView): AI Governance, Providers, and Manage Coder Agents entries. - Navbar: 'AI' entry under the admin settings dropdown, gated on viewAnyAIProvider. - Provider pages use SettingsHeader for consistent alignment with the AI Governance page; provider form fields use a side-by-side layout and the enable toggle moved from the form into the update page header alongside a disabled badge and model count. - Deployment sidebar: AI Governance and Manage Coder Agents items removed (moved to the new AI sidebar). - Submit button disabled until the form is dirty and valid. - Status column with an Enabled badge. Also restores getChatACL / updateChatACL on api.ts which were dropped in Tracey's rebase, and removes a brittle storybook timing test that no longer survives the new validateOnMount config.
|
/coder-agents-review |
|
Review posted | Chat Review history
deep-review v0.5.0 | Round 1 | Last posted: Round 1, 8 findings (1 P0, 2 P2, 2 P3, 3 Nit), COMMENT. Review Finding inventoryFindings
Contested and acknowledged(none) Round logRound 1Netero-only. 1 P0, 2 P2, 2 P3, 3 Nit. Reviewed against 57ed244..b9b0862. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
Documentation CheckNew Documentation Needed
Automated review via Coder Agents |
There was a problem hiding this comment.
First-pass review (Netero). This is a mechanical scan only; the full review panel has not yet reviewed this PR and will review after these findings are addressed.
The AI settings page restructuring is a solid step toward a dedicated admin section. Good test coverage overall with play functions exercising interactive paths, and the provider form mapping has thorough unit tests.
1 P0, 2 P2, 2 P3, 3 Nit.
The P0 is a rebase artifact: two ExperimentalApiMethods entries (getChatACL, updateChatACL) were deleted, breaking tsc and cascading into 4 CI failures. That needs resolving before anything else.
"Three existing callers pass condensed... This produces a React warning: Warning: Received 'true' for a non-boolean attribute 'condensed'." (Netero, on the PageHeaderSubtitle refactor)
🤖 This review was automatically generated with Coder Agents.
| @@ -3179,20 +3206,6 @@ class ExperimentalApiMethods { | |||
| ); | |||
There was a problem hiding this comment.
P0 [CRF-1] getChatACL and updateChatACL were deleted from ExperimentalApiMethods, but callers remain across chats.ts, chats.test.ts, api.test.ts, and ChatSharingPopover.stories.tsx. This breaks tsc --noEmit with 8 TS2551 errors and is the root cause of the check-build, lint, test-js, and chromatic CI failures. Looks like a rebase artifact. Fix: restore the two deleted methods, or update all callers. (Netero)
🤖
| condensed?: boolean; | ||
| } | ||
| }; | ||
|
|
||
| export const PageHeaderSubtitle: FC<PageHeaderSubtitleProps> = ({ |
There was a problem hiding this comment.
P2 [CRF-2] PageHeaderSubtitle now spreads ...props onto <h2>, but condensed is declared in the type and never destructured out. It leaks to the DOM. Three callers pass condensed (StarterTemplatePageView.tsx:64, TemplatePageHeader.tsx:270, TemplatePageHeader.tsx:277), producing a React warning. Fix: destructure condensed before the spread:
export const PageHeaderSubtitle: FC<PageHeaderSubtitleProps> = ({
children,
className,
condensed,
...props
}) => {(Netero)
🤖
| path="ai-governance" | ||
| element={<AIGovernanceSettingsPage />} | ||
| /> | ||
|
|
There was a problem hiding this comment.
P2 [CRF-3] The old /deployment/ai-governance route was removed with no redirect to the new /ai/settings/governance. Any bookmarks, docs, or shared links referencing the old path will 404. Could we add a <Navigate> redirect under the deployment routes?
<Route path="ai-governance" element={<Navigate to="/ai/settings/governance" replace />} />(Netero)
🤖
| form.resetForm({ values: form.values }); | ||
| } | ||
| previousIsLoading.current = isLoading; | ||
| }, [isLoading, submitError, form]); |
There was a problem hiding this comment.
P3 [CRF-4] form (from useFormik) returns a new object reference each render. Including it in the useEffect dependency array means this effect fires every render. The previousIsLoading.current guard prevents resetForm from looping, but the effect body still runs needlessly. Could extract form.resetForm into a ref, or remove form from the dep array with an eslint-disable comment explaining why. (Netero)
🤖
| type PageHeaderTitleProps = React.ComponentPropsWithRef<"h1">; | ||
|
|
||
| export const PageHeaderTitle: FC<PageHeaderTitleProps> = ({ | ||
| children, |
There was a problem hiding this comment.
P3 [CRF-5] title is destructured from props but never used in the JSX. Since title is a valid HTML attribute (native tooltip), a caller passing title="..." would expect it on the <h1>. Instead it vanishes. If the intent is to suppress the tooltip, a comment would help. Otherwise, remove it from the destructuring so it passes through via ...props. (Netero)
🤖
| onFocus?: () => void; | ||
| }; | ||
|
|
||
| export const CredentialField: React.FC<CredentialFieldProps> = ({ |
There was a problem hiding this comment.
Nit [CRF-6] New 84-line component with no dedicated Storybook story. It is exercised indirectly through ProviderForm.stories.tsx, but a standalone story would document the focus-to-clear, error state, and description rendering behaviors. (Netero)
🤖
| SettingsSidebarNavItem as SidebarNavItem, | ||
| } from "#/components/Sidebar/Sidebar"; | ||
|
|
||
| const AISettingsSidebarView: React.FC = () => { |
There was a problem hiding this comment.
Nit [CRF-7] New sidebar component with no Storybook story. Other sidebar views (e.g., DeploymentSidebarView) have stories. (Netero)
🤖
| export default meta; | ||
| type Story = StoryObj<typeof AddProviderPageView>; | ||
|
|
||
| export const AddAnthropic: Story = { |
There was a problem hiding this comment.
Nit [CRF-8] The three AddProviderPageView stories (AddAnthropic, AddOpenAI, AddBedrock) are visual-only with no play functions. The page has interactive logic (form submission, error handling, navigation on success); per project convention, page stories should include play functions asserting behavior. (Netero)
🤖
Rebases the frontend changes from #25328 onto current
mainand adds UI improvements for the new top-level AI admin section.What changed
Top-level AI section at
/ai/settingswith its own layout and sidebar:/ai/settings/governance(deployment config, wrapped inDeploymentConfigProvider)/ai/settings(index route, provider list)/agents/settings/agentsNavbar: Admin settings dropdown now has an "AI" entry pointing to
/ai/settings, gated onviewAnyAIProvider.Provider pages use
SettingsHeaderfor consistent alignment with the AI Governance page. Provider form fields use a cleaner side-by-side layout. The enable toggle moved from the form into the update page header alongside a disabled badge and model count. Providers description updated with BYOK docs link.Deployment sidebar: AI Governance and Manage Coder Agents items removed (moved to AI sidebar).
Unsaved changes prompt: Ported from #25328. Warns before leaving a dirty provider form via
useBlockerandbeforeunload.Supersedes #25328. Companion PR #25574 covers the agents sidebar change (separate reviewer).
Implementation details
Files
AISettingsLayout/AISettingsSidebarView— new sidebar with 3 itemsDeploymentSidebarView— removed AI itemsDeploymentDropdown/Navbar/NavbarView— restoredcanViewAISettingspropProvidersPageView—SettingsHeader,AddProviderDropdowncomponent,TableEmptyCTA, BYOK linkAddProviderPageView—SettingsHeaderTitle, streamlined layoutUpdateProviderPageView—SettingsHeaderTitle, enable toggle in header, model settings link, delete buttonProviderForm— side-by-side fields, removed enable toggle, unsaved-changes prompt, submit disabled until dirty+validuseUnsavedChangesPrompt— new hook (ported from feat: implement AI settings page with providers management UI #25328)router.tsx—/ai/settingsroutes withDeploymentConfigProviderwrapper for governancepermissions/index.ts—viewAnyAIProviderincanViewDeploymentSettings()Context from original PR
The original PR (#25328 by @jakehwll) was stacked on top of the providers-handlers PRs and accumulated 84 commits. This PR extracts the frontend-only files, applies them cleanly on current
main, and adds the UI refinements.