fix(dashboard): expose Modal Base URL field in connection modals (#12704) - #12736
Conversation
|
Follow-up commit: the new modal case pushed getProviderBaseUrlPlaceholder from 15 to 16 against the complexity cap, so I moved the literal placeholder examples into a record lookup keyed by provider id instead of a switch. Same values, no behavior change; the modal and kimi suites still pass. |
|
Great fix — clean root-cause diagnosis, and I really appreciate the follow-up commit where you |
…als (diegosouzapw#12704) Modal is bring-your-own-deploy, so every connection needs its own app URL, and the server-side validator already required providerSpecificData.baseUrl. The add/edit connection form never rendered the field, so a Modal connection could not be validated or saved at all. Adding the id to CONFIGURABLE_BASE_URL_PROVIDERS reuses the same always-on Base URL field as the kimi/moonshot case (diegosouzapw#7447). The placeholder switch is folded into a record lookup in the same commit so the function stays under the complexity cap as ids are added; the record was checked against the switch for every pre-existing id and only "modal" changes behaviour. Rebased onto the current release tip, which now carries diegosouzapw#13120's own entry in the same set.
9665926 to
35e332c
Compare
|
Rebased onto the current tip. The conflict was Adding the modal case also pushed Same three assertions as before: modal is base-URL configurable, the placeholder shows the app URL shape, and the server-side validator still requires the URL. Two of the three fail without the change. All 139 tests across the suites importing |
|
Thanks @gonisulaimann — merging via the release merge-train. Validated in local merge-train (.claude/worktrees/merge-train-20260918-111718-suite.log) on the devbox @ train tip 7bb373fba5e241964c0ffb17bd03a804700839bb, boarded with 55 sibling PRs: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; 747/747 changed-area node:test cases + 476/476 vitest green (fast parity mode — the full suite ran today on the release tip via the base-red train and runs again on the 3b train). Merged --admin per merge-gates §7. |
0551893
into
diegosouzapw:release/v3.8.51
…als (diegosouzapw#12704) (diegosouzapw#12736) Modal is bring-your-own-deploy, so every connection needs its own app URL, and the server-side validator already required providerSpecificData.baseUrl. The add/edit connection form never rendered the field, so a Modal connection could not be validated or saved at all. Adding the id to CONFIGURABLE_BASE_URL_PROVIDERS reuses the same always-on Base URL field as the kimi/moonshot case (diegosouzapw#7447). The placeholder switch is folded into a record lookup in the same commit so the function stays under the complexity cap as ids are added; the record was checked against the switch for every pre-existing id and only "modal" changes behaviour. Rebased onto the current release tip, which now carries diegosouzapw#13120's own entry in the same set. Co-authored-by: Goni Sulaiman <gonisulaimann@users.noreply.github.com>
Fixes #12704.
Modal is bring-your-own-deploy — every user runs their model on a unique endpoint (
https://<workspace>--<app>.modal.run/v1), so there's no fixed host to preset. The server-side validator (src/lib/providers/validation.ts) already requiresproviderSpecificData.baseUrlfor modal, but the add/edit connection modals only rendered Token ID / Token Secret, so there was no way to fill the required field and every Modal connection failed validation.This adds
modaltoCONFIGURABLE_BASE_URL_PROVIDERS, the same mechanism kimi/moonshot use (#7447). Both modals key their Base URL field off that one predicate (AddApiKeyModal.tsx:1104,EditConnectionModal.tsx:199), and Save stays disabled while the field is empty for a provider with no default URL (AddApiKeyModal.tsx:1160), so a Modal connection now renders the field, requires it, and validates it on save.Rebased onto the current tip. #13120 added
agnesto the same set and to the same placeholder function while this was open; both changes are additive, so the merge keeps its entry and mine.One thing worth flagging: adding a case pushed
getProviderBaseUrlPlaceholderover the complexity cap, so it now reads from a record lookup plus a small set for the ids whose placeholder is their configured default URL. I compared it against the switch it replaces for every id that switch handled, including the ones #13120 added, plus null/undefined/unknown, andmodalis the only value that changes.Tests:
tests/unit/modal-base-url-field-12704.test.tsasserts modal is base-URL configurable, the placeholder shape, and that the server-side validator still rejects a baseUrl-less modal connection, so the UI and the backend can't drift apart. Two of the three fail without the fix. All 139 tests in the suites that importproviderPageHelpersstay green; typecheck, lint and prettier pass.