refactor(providers): rename managed→platform for catalog auth symbols#30432
Conversation
c852f3d to
eea27ee
Compare
There was a problem hiding this comment.
💡 Codex Review
Calling loadUserPlugins() without first installing globalThis.__vellumPluginRuntime breaks the documented legacy plugin contract: register-based plugins that read the runtime in module scope (for example assistant/examples/plugins/echo/register.ts, as described in assistant/src/plugins/external-api.ts) will throw during import and never register, so existing workspace plugins silently stop working.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "runtime/routes/credential-routes.ts", // CLI credential management routes (CLI-migrated to IPC) | ||
| "runtime/routes/platform-routes.ts", // CLI platform connect/disconnect/status routes (CLI-migrated to IPC) | ||
| "ipc/skill-routes/providers.ts", // host.providers.secureKeys.getProviderKey IPC route (out-of-process SkillHost companion) | ||
| "daemon/external-plugins-bootstrap.ts", // reads credentials at plugin init (manifest.requiresCredential) via the CES-mediated getSecureKeyAsync path |
There was a problem hiding this comment.
Keep external-api in secure-keys importer allowlist
Removing this allowlist entry introduces a self-failing invariant: assistant/src/plugins/external-api.ts still imports getSecureKeyAsync from security/secure-keys.js, so the secure-keys is only imported by authorized modules scan will now always report an unauthorized importer and fail this test in normal CI/dev environments.
Useful? React with 👍 / 👎.
|
🖥️ macOS app artifact for this PR is ready for download: Download vellum-assistant-pr-30432.dmg To run this build locally: |
|
🖥️ macOS app artifact for this PR is ready for download: Download vellum-assistant-pr-30432.dmg To run this build locally: |
6e32485 to
05f86cc
Compare
|
📦 Assistant artifact for this PR is ready for download: Download assistant-pr-30432 To run this build locally: |
|
📦 Assistant artifact for this PR is ready for download: Download assistant-pr-30432 To run this build locally: |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Follow-up to the catalog-driven auth refactor (#30411 in this same branch + #6566 on the web side). The codebase has been using two words for the same concept: - User-facing UI: 'Platform (managed by Vellum)' - Auth-type wire value: 'platform' (AuthType.platform on Swift, "platform" in TS connection JSON) - Internal symbols: MANAGED_PROVIDER_META, supportsManagedAuth, isManagedCapable, managedCapableProviders, managed-proxy/ directory This commit aligns the internal symbols with the user-facing 'platform' terminology so future readers don't have to mentally translate. Renames ------- Symbols (word-boundary-safe sed across .ts/.swift/.tsx/.json): MANAGED_PROVIDER_META -> PLATFORM_PROVIDER_META supportsManagedAuth -> supportsPlatformAuth isManagedCapable -> isPlatformCapable managedCapableProviders -> platformCapableProviders Files / paths: assistant/src/providers/managed-proxy/ -> .../platform-proxy/ assistant/src/__tests__/managed-proxy-context.test.ts -> .../platform-proxy-context.test.ts assistant/src/__tests__/provider-managed-proxy-integration.test.ts -> .../provider-platform-proxy-integration.test.ts assistant/src/__tests__/secret-routes-managed-proxy.test.ts -> .../secret-routes-platform-proxy.test.ts Import-path string substitutions inside source files: './managed-proxy/...' -> './platform-proxy/...' What is NOT renamed (out of scope) ---------------------------------- * User-facing copy 'Platform (managed by Vellum)' — that's the UX label disambiguating what 'Platform' means; renaming would degrade clarity for end users. * Runtime routing-source discriminator string 'managed-proxy' — used in ProviderRoutingSource union type and several runtime checks. Leaving for a separate follow-up if Noa wants it tightened. * MANAGED_USAGE_LIMIT error code — wire-format contract with the macOS/iOS clients; out-of-scope structural rename. * ManagedProxyCredentials / ManagedProxyContext type names — left for a separate follow-up. Test ---- * bunx tsc --noEmit on assistant/ — clean. * bun test platform-proxy-context, provider-platform-proxy-integration, secret-routes-platform-proxy, llm-catalog-parity, satellite-connection-routing, credential-security-invariants — 110 tests pass. * bun run scripts/sync-llm-catalog.ts --check — clean.
The rename in the parent commit shifted import path prefixes (`managed-proxy/*` → `platform-proxy/*`), which puts them later in the alphabetical order simple-import-sort enforces. * assistant/src/__tests__/llm-catalog-parity.test.ts: swap PROVIDER_CATALOG / PLATFORM_PROVIDER_META order so `model-catalog` (m) precedes `platform-proxy/constants` (p). * assistant/src/providers/registry.ts: move the `./platform-proxy/context.js` import block after `./model-intents.js` for the same reason. Test ---- * bunx tsc --noEmit clean. * No behavior change.
e20a1c3 to
f750167
Compare
|
📦 Assistant artifact for this PR is ready for download: Download assistant-pr-30432 To run this build locally: |
|
🖥️ macOS app artifact for this PR is ready for download: Download vellum-assistant-pr-30432.dmg To run this build locally: |
Why
Follow-up to #30411 (just merged) and the macOS/web UI changes (#30408, vellum-ai/vellum-assistant-platform#6566). The codebase has been using two words for the same concept:
"platform"(AuthType.platform/ connection JSON)MANAGED_PROVIDER_META,supportsManagedAuth,isManagedCapable,managedCapableProviders,managed-proxy/This PR aligns the internal symbols with the user-facing platform terminology so future readers don't have to mentally translate.
What
Single commit on top of current
main(which now includes #30411's catalog flag).Symbol renames (word-boundary-safe sed across .ts/.swift/.tsx/.json)
MANAGED_PROVIDER_METAPLATFORM_PROVIDER_METAsupportsManagedAuthsupportsPlatformAuthisManagedCapableisPlatformCapablemanagedCapableProvidersplatformCapableProvidersPath renames
assistant/src/providers/managed-proxy/assistant/src/providers/platform-proxy/assistant/src/__tests__/managed-proxy-context.test.ts.../platform-proxy-context.test.tsassistant/src/__tests__/provider-managed-proxy-integration.test.ts.../provider-platform-proxy-integration.test.tsassistant/src/__tests__/secret-routes-managed-proxy.test.ts.../secret-routes-platform-proxy.test.tsImport-path strings inside source
'./managed-proxy/...'→'./platform-proxy/...'(and equivalent forms).What is NOT renamed (out of scope)
"managed-proxy"— used in theProviderRoutingSourceunion type and several runtime=== "managed-proxy"checks. Pure in-process, no wire/persistence concern, but kept out of this PR to keep the rename mechanical and contained. Easy follow-up if you want it tightened.MANAGED_USAGE_LIMITerror code — wire-format contract with the macOS/iOS clients; out-of-scope structural rename.ManagedProxyCredentials/ManagedProxyContexttype names — left for a separate follow-up.Test
bunx tsc --noEmitonassistant/— clean (only pre-existing unrelatedpackages/errors).bun teston platform-proxy-context, provider-platform-proxy-integration, secret-routes-platform-proxy, llm-catalog-parity, satellite-connection-routing, credential-security-invariants — 110 pass / 0 fail.bun run scripts/sync-llm-catalog.ts --check— clean.Followups (if scope creep wanted)
"managed-proxy"discriminator andManagedProxy*type names — bounded, in-process, safe.vellum-ai/vellum-assistant-platform#6581will get its parallel rename (providerSupportsManagedAuth,PROVIDER_SUPPORTS_MANAGED_AUTH) once this merges and the catalog re-vendors.