-
Notifications
You must be signed in to change notification settings - Fork 89
fix(environments): env-seed fallback for getPlatformUrl, revert H1 #25595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ import { join } from "path"; | |
| import { | ||
| resolveTargetAssistant, | ||
| saveAssistantEntry, | ||
| syncConfigToLockfile, | ||
| } from "../lib/assistant-config.js"; | ||
| import { dockerResourceNames, wakeContainers } from "../lib/docker.js"; | ||
| import { isProcessAlive, stopProcessByPidFile } from "../lib/process"; | ||
|
|
@@ -183,21 +182,17 @@ export async function wake(): Promise<void> { | |
| } | ||
| } | ||
|
|
||
| // Set BASE_DATA_DIR so ngrok and the config→lockfile sync read the | ||
| // correct instance's workspace config. Waking a different assistant must | ||
| // refresh the lockfile's top-level platformBaseUrl so getPlatformUrl() — | ||
| // and downstream code like `vellum login` and ensureRegistration — targets | ||
| // the tenant the woken assistant is configured for. | ||
| // Auto-start ngrok if webhook integrations (e.g. Telegram) are configured. | ||
| // Scope BASE_DATA_DIR to the woken instance so ngrok reads the correct | ||
| // instance config, then restore on any exit path. | ||
| const prevBaseDataDir = process.env.BASE_DATA_DIR; | ||
| process.env.BASE_DATA_DIR = resources.instanceDir; | ||
| try { | ||
| // Auto-start ngrok if webhook integrations (e.g. Telegram) are configured. | ||
| const ngrokChild = await maybeStartNgrokTunnel(resources.gatewayPort); | ||
|
Comment on lines
188
to
191
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The wake path still scopes Useful? React with 👍 / 👎. |
||
| if (ngrokChild?.pid) { | ||
| const ngrokPidFile = join(resources.instanceDir, ".vellum", "ngrok.pid"); | ||
| writeFileSync(ngrokPidFile, String(ngrokChild.pid)); | ||
| } | ||
| syncConfigToLockfile(); | ||
| } finally { | ||
| if (prevBaseDataDir !== undefined) { | ||
| process.env.BASE_DATA_DIR = prevBaseDataDir; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this change,
vellum useonly callssetActiveAssistant(name)and no longer refreshesplatformBaseUrlfrom the selected assistant's workspace config, sogetPlatformUrl()can keep returning the previously persisted tenant URL. This is user-visible when assistants in one lockfile have differentplatform.baseUrlvalues (e.g. created/edited via config overrides): switching active assistants then running platform-facing commands (login/registration/upgrade paths that callgetPlatformUrl) can target the wrong host.Useful? React with 👍 / 👎.