fix(ci): staging builds resolve to prod API URL — bake VITE vars into build.yml#1371
fix(ci): staging builds resolve to prod API URL — bake VITE vars into build.yml#1371YellowSnnowmann wants to merge 4 commits intotinyhumansai:mainfrom
Conversation
build.yml runs on every PR and push to main. Without VITE_OPENHUMAN_APP_ENV and VITE_BACKEND_URL, config.ts resolves APP_ENV as undefined and DEFAULT_BACKEND_URL falls through to https://api.tinyhumans.ai (production). Set both vars to the staging values so all CI build artifacts point at staging-api.tinyhumans.ai. Release builds (release-staging.yml → build-desktop.yml) already pass the correct values per environment and are unaffected. Closes tinyhumansai#1370
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRuntime resolvers for app environment and backend URL now prefer runtime env vars (including VITE_), fall back to compile-time option_env!, and have updated docs and unit tests; the CI Tauri build step now bakes staging VITE_ env vars ( ChangesStaging Build Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
…n_env!
Root cause of staging DMG hitting prod:
- effective_api_url resolves the URL at runtime via std::env::var.
- When a user runs the installed DMG there is no shell environment, so
OPENHUMAN_APP_ENV and BACKEND_URL are never set.
- The function fell through to DEFAULT_API_BASE_URL (production) even
for staging builds.
Fix: add compile-time option_env! fallbacks to both app_env_from_env()
and api_base_from_env(). build-desktop.yml already sets OPENHUMAN_APP_ENV,
VITE_OPENHUMAN_APP_ENV, BACKEND_URL, and VITE_BACKEND_URL as build env
vars — option_env! captures them at cargo build time and bakes them into
the binary, so the installed app resolves the correct URL with no shell.
This mirrors the pattern the Tauri shell already uses for its Sentry
environment tag (option_env!("VITE_OPENHUMAN_APP_ENV") in lib.rs).
Resolution order is now:
1. config.api_url (user explicitly set it)
2. BACKEND_URL / VITE_BACKEND_URL runtime env var
3. BACKEND_URL / VITE_BACKEND_URL baked in at compile time ← new
4. OPENHUMAN_APP_ENV runtime → env-aware default URL
5. OPENHUMAN_APP_ENV baked in at compile time → env-aware default ← new
6. Hard fallback: api.tinyhumans.ai
Closes tinyhumansai#1370
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/api/config.rs (1)
35-41: ⚡ Quick winAdd branch-level debug/trace diagnostics for env resolution paths.
These new resolver branches (runtime vs compile-time vs default fallback) are operationally important but currently silent, which will make future staging/prod misrouting incidents harder to triage. Prefer stable prefixes and avoid logging raw URL values or secrets.
As per coding guidelines "Default to verbose diagnostics on new/changed flows with entry/exit logging, branches, external calls, retries/timeouts, state transitions, and errors; use stable grep-friendly prefixes and correlation fields; never log secrets or full PII".
Also applies to: 64-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/config.rs` around lines 35 - 41, Add branch-level, non-sensitive diagnostics to the API resolver: in api_base_from_env (and the related resolver around lines 64-70) add entry and exit trace/debug calls and a short, stable-prefix branch log for each resolution path (runtime env branch, compile-time option_env! branch, and the final default) using a tracing macro (e.g., trace!/debug!) with messages like "CONFIG:API_BASE:BRANCH=runtime", "CONFIG:API_BASE:BRANCH=compile_time", "CONFIG:API_BASE:BRANCH=default" and include a boolean or masked indicator (e.g., value_present=true/false or "<masked>") rather than the raw URL; ensure logs are emitted on both success and fallback paths and use the same prefix for easy grepping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/config.rs`:
- Around line 72-80: This file has rustfmt/prettier formatting drift around the
environment-var parsing block (references: APP_ENV_VAR, VITE_APP_ENV_VAR,
option_env!("OPENHUMAN_APP_ENV"), option_env!("VITE_OPENHUMAN_APP_ENV")) and
later at lines near 146–152; run the project's formatter (cargo fmt) and any
JS/Prettier checks, then stage the formatted changes so the CI formatting checks
pass; ensure you do not change logic in functions using those symbols—only
reformat whitespace/line breaks to match rustfmt output and re-run the CI
locally before pushing.
- Around line 125-137: Wrap the tests app_env_from_env_reads_runtime_var and
api_base_from_env_reads_runtime_var with a shared test-wide Mutex to prevent
concurrent env mutations; follow the pattern used in
tests/webview_apis_bridge.rs by creating or reusing a static lazy Mutex guard
(e.g., TEST_ENV_LOCK) and acquiring it at the start of each test before
setting/restoring OPENHUMAN_APP_ENV or API_BASE env vars so the existing
save-restore logic remains but runs under the lock to avoid race conditions.
- Around line 43-47: The runtime and compile-time env selection currently uses
chaining (std::env::var(...).or_else(...) and option_env!(...).or(...)) which
treats an empty string as a valid value and thus blocks fallback keys; update
both places to iterate over the candidate keys
(["BACKEND_URL","VITE_BACKEND_URL"] for the API URL and the app env pair used in
app_env_from_env) and return the first key whose value exists and is non-empty
(for runtime, call std::env::var for each key and normalize_api_base_url on the
candidate before filtering out empty; for compile-time, check option_env! for
each key in order and pick the first Some that is non-empty) so empty values
don’t prevent checking the next candidate.
---
Nitpick comments:
In `@src/api/config.rs`:
- Around line 35-41: Add branch-level, non-sensitive diagnostics to the API
resolver: in api_base_from_env (and the related resolver around lines 64-70) add
entry and exit trace/debug calls and a short, stable-prefix branch log for each
resolution path (runtime env branch, compile-time option_env! branch, and the
final default) using a tracing macro (e.g., trace!/debug!) with messages like
"CONFIG:API_BASE:BRANCH=runtime", "CONFIG:API_BASE:BRANCH=compile_time",
"CONFIG:API_BASE:BRANCH=default" and include a boolean or masked indicator
(e.g., value_present=true/false or "<masked>") rather than the raw URL; ensure
logs are emitted on both success and fallback paths and use the same prefix for
easy grepping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- api_base_from_env and app_env_from_env now iterate each key
independently so an empty BACKEND_URL / OPENHUMAN_APP_ENV does not
shadow a valid VITE_BACKEND_URL / VITE_OPENHUMAN_APP_ENV. Previously
the .or_else / .or chain only tried the secondary key on Err (missing),
not on Ok("") (present-but-empty).
- Add ENV_LOCK (OnceLock<Mutex>) to serialise the two env-mutating tests,
preventing flaky failures under parallel test execution.
- Add two new tests covering the empty-primary fall-through path.
Summary
Two fixes for staging builds resolving to prod API URL.
Fix 1 —
build.yml(CI validation builds)build.ymlhad noVITE_OPENHUMAN_APP_ENVorVITE_BACKEND_URLset.config.tsresolvedAPP_ENVasundefinedandDEFAULT_BACKEND_URLfell through tohttps://api.tinyhumans.ai. Every PR CI build artifact baked in the prod URL.Fix 2 —
src/api/config.rs(the real bug — shipped DMG hitting prod)The Rust core resolved API URL purely at runtime via
std::env::var. When a user runs the installed DMG there is no shell environment, soOPENHUMAN_APP_ENVandBACKEND_URLare never set, andeffective_api_urlfell through toDEFAULT_API_BASE_URL(production) even for staging builds.Added
option_env!compile-time fallbacks toapi_base_from_env()andapp_env_from_env().build-desktop.ymlalready setsOPENHUMAN_APP_ENV,VITE_OPENHUMAN_APP_ENV,BACKEND_URL, andVITE_BACKEND_URLas build env vars —option_env!captures them atcargo buildtime and bakes them into the binary. This mirrors the pattern the Tauri shell already uses for its Sentry environment tag.Resolution order after fix:
config.api_url(user explicitly set it)BACKEND_URL/VITE_BACKEND_URLruntime env varBACKEND_URL/VITE_BACKEND_URLbaked in at compile time ← newOPENHUMAN_APP_ENVruntime → env-aware default URLOPENHUMAN_APP_ENVbaked in at compile time → env-aware default ← newapi.tinyhumans.aiTest plan
cargo checkpasses cleanapi::configunit tests added for runtime var readsoption_env!path is verified by the build workflow setting the vars — no unit test can coveroption_env!since it is resolved at compile time, not runtimebuild.ymlenv var change is a one-line CI config additionCloses #1370
Summary by CodeRabbit