refactor(web): split provider-editor-modal into focused modules (LUM-2224)

[devin-ai-integration]

Prompt / plan

LUM-2224 — Phase 2 of the settings/AI domain decomposition. Split provider-editor-modal.tsx (910 lines) into focused, single-responsibility modules, then audit every extraction from first principles.

What changed

New files:

• provider-editor-constants.ts (55 lines) — Types (AuthType), constants (CONNECTION_PROVIDERS, AUTH_TYPE_DISPLAY_NAMES), and pure helpers (parseCredentialRef, connectionSaveErrorMessage). Zero React dependencies.

• use-provider-credentials-list.ts (77 lines) — TanStack Query hook wrapping secretsGet() with org-readiness gating (useIsOrgReady()), retry logic (shouldRetryDaemonError), and Sentry error reporting. Query key is module-private (implementation detail of the hook, not a shared constant).

• provider-editor-api-key-section.tsx (201 lines) — Sub-component encapsulating the API Key input field + Advanced credential-reference disclosure. Owns its own UI state (isAdvancedExpanded, isCreatingNewCredential, newCredentialName). Parent passes values and callbacks via props.

Modified:

• provider-editor-modal.tsx (910 → 662 lines, −248) — Imports from the three new modules. Inline constants, credential query, and API key JSX replaced with extracted abstractions. Parent retains apiKeyValue and credential state (needed by save handler).

Design decisions

• State ownership split: Parent owns apiKeyValue, credential, isSavingKey (all flow into the save handler). Sub-component owns isAdvancedExpanded, isCreatingNewCredential, newCredentialName (pure UI disclosure state with no save-handler dependency).
• Query key is hook-private: PROVIDER_CREDENTIALS_LIST_QK is an implementation detail of useProviderCredentialsList, not a shared constant. It lives in the hook file, not the constants file.
• Constants file is pure: No React, no hooks, no side effects. Importable by tests and other modules without pulling in React.

First-principles audit fixes (second commit)

1. DRY credential parsing in save handler — The save handler manually re-implemented parseCredentialRef's credential/service/field splitting logic. Replaced with a call to the extracted helper, eliminating the duplication.
2. Stale comments — Updated comments in both provider-editor-modal.tsx and provider-editor-api-key-section.tsx that still referenced isCreatingNewCredential as a condition for shouldShowAdvancedSection after it was moved to the sub-component.
3. Duplicate imports — Combined two separate import type lines from the same module in provider-editor-api-key-section.tsx.
4. /// comment style normalization — All 25 triple-slash comments in apps/web/src/ were in this one file (a Swift convention). Normalized to // for consistency with the rest of the codebase.
5. Removed cross-file references in comments — Removed Mirrors X in profile-editor-modal.tsx references that describe refactoring lineage rather than what the code does.

Test plan

• 57/57 domain tests pass (bun test src/domains/settings/ai/)
• Lint clean on all 4 changed files
• Typecheck clean (pre-existing 11 errors in oauth-apps.ts/oauth-providers.ts confirmed on main — unrelated)
• Behavioral equivalence: traced all 4 consumer code paths (create, edit, managed-edit, oauth_subscription) — identical behavior before and after
• CI 7/7 green

Closes LUM-2224

Link to Devin session: https://app.devin.ai/sessions/4b3b130bbf274e5487da3b4992883093
Requested by: @ashleeradka

[linear]

LUM-2224

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

[ashleeradka]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Nice work!

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[vex-assistant-bot]

✦ APPROVE

Value: Phase 2 of the settings/AI decomposition arc (sibling to #33246 which landed ~50 min ago). provider-editor-modal.tsx goes 910 → 662 (−248) by extracting three single-responsibility modules. Every extracted module is testable in isolation, has zero unintended coupling back to the modal, and useStoredCredentialPresence from #33246 is correctly retained at the call site.

What this does:

1. provider-editor-constants.ts (55 lines, zero React) — types (AuthType), constants (CONNECTION_PROVIDERS, AUTH_TYPE_DISPLAY_NAMES), pure helpers (parseCredentialRef, connectionSaveErrorMessage). Importable by tests without dragging React in.
2. use-provider-credentials-list.ts (77 lines) — TanStack hook wrapping secretsGet. Honors the full daemon-config-hooks contract: useIsOrgReady gate, shouldRetryDaemonError, captureError(bestEffort), useMemo'd tuple queryKey [QK, assistantId], staleTime: 30_000. Query key is module-private (correctly — it's an implementation detail, not a shared constant).
3. provider-editor-api-key-section.tsx (201 lines) — sub-component owning ONLY the disclosure UI state (isAdvancedExpanded, isCreatingNewCredential, newCredentialName). Parent retains apiKeyValue / credential / isSavingKey because those flow into the save handler. Clean ownership boundary.
4. provider-editor-modal.tsx — replaces inline definitions with imports; second commit (a554624) self-addresses Devin's stale-comment finding and DRYs the save handler's manual credential-ref parsing onto parseCredentialRef from the new constants module.

Anti-pattern cross-check

• No as runtime-boundary casts in any of the 4 files. ✓
• useStoredCredentialPresence retained at the modal call site (2 occurrences) — #33246's hook is NOT dropped. ✓
• daemon-config-hooks.md compliance on useProviderCredentialsList: useIsOrgReady ✓, shouldRetryDaemonError ✓, captureError(bestEffort: true) ✓, assertHasResponse + response.ok gate before data! ✓, useMemo'd query-key tuple ✓.
• data!.secrets ?? data!.accounts non-null assertions are post-assertHasResponse + !response.ok throws — safe.
• Semantic tokens used throughout (var(--content-tertiary), var(--content-secondary), var(--system-negative-strong) is unchanged from main); text-body-small-default typography token. design-system-review.md compliant. ✓
• State ownership correctly partitioned: pure UI disclosure state in sub-component, save-handler-bound state in parent. No prop drilling of state that doesn't belong to the parent.
• No SSE/seq territory. No vembda territory. (R11e clean — Vargas + Mahmoud untouched.)

Devin's first-commit finding (auto-fixed at HEAD)

At 03fc2c7b Devin flagged: "Stale comment describes removed isCreatingNewCredential condition in shouldShowAdvancedSection" — the comment at lines 400–402 still referenced a condition that had been moved into the sub-component.

Devin self-fixed in a554624: updated the parent comment to describe only the two remaining conditions, and the sub-component's comment now delegates to the parent's showAdvancedSection prop. Verified at HEAD. The second commit also handled 4 other first-principles audit items (DRY save handler onto parseCredentialRef, dedupe import type lines, normalize /// Swift-style comments to //, drop cross-file refactoring-lineage comments).

Self-improvement worth calling out: Devin's own "audit fixes" second commit catching /// triple-slash Swift comments (25 total, all in this one file — clearly drifted in from someone's Swift muscle memory) is exactly the kind of first-principles housekeeping that should ride along with extraction PRs. The lineage-comment cleanup ("Mirrors X in profile-editor-modal.tsx") is also right — those describe how the code got here, not what it does.

Merge gate at HEAD a554624: Vex ✓ · Codex 👍 ("Didn't find any major issues. Nice work!") + 👍 reaction on PR description ✓ · Devin: 1 issue found at first commit, self-fixed at HEAD with inline clearance ✓ · CI 7/7 green ✓. Will squash-merge.

Vellum Constitution — Distinct: a clean decomposition where the parent's line count drops sharply but the new files each have a single, easy-to-name reason to exist — and the second commit's self-audit (Swift comment style drift, lineage refs, save-handler DRY) shows the kind of taste that compounds when the next person reads the diff.