Centralize environment configuration with validation

[garciadias]

Description

Refactors environment variable handling by introducing a centralized configuration module (config.ts) that validates required variables at application startup and provides a single source of truth for all configuration values.

Key Changes

• New config.ts module: Centralizes all environment variable reading and validation with fail-fast behavior for missing required variables
• Removed hardcoded config: Deletes window.js which contained hardcoded configuration values
• Updated all consumers: Migrates auth.ts, api.ts, ModelUpload.vue, and main.ts to use getConfig() instead of process.env or window globals
• Enhanced documentation: Updates README with clear distinction between required and optional variables, with defaults and troubleshooting guidance
• Comprehensive test coverage: Adds 156 lines of unit tests covering validation, defaults, and edge cases
• Improved error handling: Application displays configuration errors in the browser before attempting to initialize

Benefits

• Fail-fast validation: Missing required variables are caught immediately at startup with clear error messages
• Single source of truth: All configuration logic in one place, reducing duplication and inconsistency
• Type safety: AppConfig interface provides TypeScript support throughout the application
• Better DX: Clear error messages guide developers to set missing variables in .env.development
• Testability: Configuration can be easily mocked in tests via vi.stubEnv()

Linked Issues

Fixes #

Checklist

• Follows the project's coding conventions and style guide
• Updates documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published

Type of Change

• Non-breaking change (fix or new feature that would not break existing functionality).
• New tests added to cover the changes.
• In-line docstrings updated.
• Documentation updated.

Testing

Added comprehensive unit tests in config.spec.ts covering:

• Required variable validation (individual and collective)
• Error message guidance
• Optional variable defaults
• Type coercion for numeric values
• Boolean parsing for local mode flag

All tests pass locally. Configuration validation is tested in isolation and integration with existing test setup ensures no regressions.

https://claude.ai/code/session_01Fi3u1Mvm2NDBXmx5DMNdjj

[codecov]

Codecov Report

❌ Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
flip-ui/src/partials/models/ModelUpload.vue	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR centralizes flip-ui environment configuration into a single validated getConfig() module, migrating key consumers off scattered process.env/window usage and improving startup failure visibility and documentation.

Changes:

• Added src/utils/config.ts with required/optional env validation + typed AppConfig
• Migrated config consumers (auth.ts, api.ts, ModelUpload.vue, main.ts) to use getConfig()
• Updated docs and examples (flip-ui/README.md, .env.development.example) and added unit tests for config behavior

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
flip-ui/test/setup.ts	Seeds env values for tests so config validation doesn’t fail globally
flip-ui/src/utils/config.ts	New centralized config reader/validator returning typed AppConfig
flip-ui/src/utils/auth.ts	Switched Cognito/local-mode logic to read from getConfig()
flip-ui/src/utils/tests/config.spec.ts	New unit tests covering required/optional env parsing and errors
flip-ui/src/services/api.ts	Uses config-derived base URL for Axios client
flip-ui/src/partials/models/ModelUpload.vue	Uses config-derived blacklist list source
flip-ui/src/main.ts	Attempts to show configuration errors early and uses config for local mode
flip-ui/src/env.d.ts	Removes window.* runtime config globals typing
flip-ui/README.md	Documents required vs optional env vars and troubleshooting
flip-ui/public/js/window.js	Removes hardcoded runtime config values
.env.development.example	Adds required/optional env variable guidance and placeholders

Comments suppressed due to low confidence (1)

flip-ui/public/js/window.js:13

• window.js has been emptied but not deleted. Since it is still referenced by flip-ui/index.html, keeping an empty file adds an unnecessary request; deleting it will require updating index.html to remove the script tag. Either remove both (preferred) or keep a small stub with a comment explaining why it remains.

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

[Copilot]

authConfig is built from _config = getConfig() at module load time. If required env vars are missing, this will throw during import (before main.ts can catch and render the configuration error), defeating the intended fail-fast UI. Consider exporting a factory (e.g., getAuthConfig()), or building authConfig in main.ts after getConfig() succeeds (or passing AppConfig into an auth-config builder).

[Copilot]

let appConfig; introduces an implicit any under strict TypeScript settings, which can fail typechecking/linting. Prefer typing it (e.g., let appConfig: AppConfig) or use a typed const appConfig = getConfig() inside the try block.

[Copilot]

Directly mutating import.meta.env[...] is brittle in Vite/Vitest (and may be typed as read-only). Prefer using vi.stubEnv() for these defaults (and potentially vi.unstubAllEnvs() in teardown) so env setup is consistent and easily overridden per test.

[atriaybagur]

Happy to take a look at this when I do the S3+Cloudfront implementation --

⏺ Here's the key finding:

  The PR as written is NOT compatible with a future S3+CloudFront deployment.

  The core issue: import.meta.env values are baked in at build time by Vite. Today this works because the production Dockerfile
  runs npm run dev (the Vite dev server injects env vars dynamically). But with S3+CloudFront serving static assets, you'd need
  to rebuild for every environment — you can't deploy the same artifact to staging and prod.

  Ironically, the old window.js that was deleted was the right pattern for S3+CloudFront (runtime config injection via a
  separate script). The implementation was poor (hardcoded values, globals), but the concept is what you need.

  Recommended fix: A hybrid approach where config.ts reads from window.__FLIP_CONFIG__ first (runtime, for static hosting) and
  falls back to import.meta.env (build-time, for Vite dev server). This preserves the PR's improvements (validation, typing,
  AppConfig interface) while keeping the door open for S3+CloudFront.

plan file

 Here is Claude's plan:                                                                                                         
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌
 PR #228 Review: Centralize Environment Configuration with Validation                                                           
                                                                  
 Context

 PR #228 replaces the old window.js approach (which set globals on window at runtime) with a new config.ts module that reads
 from Vite's import.meta.env and validates required variables at startup. The user asked whether these changes are compatible
 with a future S3+CloudFront static hosting refactor.

 Critical Finding: S3+CloudFront Compatibility

 The new import.meta.env approach is NOT compatible with S3+CloudFront static hosting.

 Why it works today

 The production Dockerfile (flip-ui/Dockerfile, line 41) runs npm run dev — the Vite dev server — even in production. This
 means import.meta.env variables are injected dynamically at runtime by the dev server, not baked into static assets. The
 current deployment is EC2-based with the Vite dev server running inside a Docker container.

 Why it breaks with S3+CloudFront

 When you build static assets (npm run build), Vite replaces all import.meta.env.VITE_* references with their literal values at
  build time. The resulting JS bundles in S3 contain hardcoded strings. This means:

 - You must rebuild for each environment (dev/staging/prod)
 - You cannot deploy the same artifact to multiple environments
 - Configuration changes require a full rebuild and redeployment

 The old window.js was actually the right pattern

 The deleted window.js was a runtime configuration injection pattern — a separate file served alongside the SPA that could be
 swapped or generated per-environment without rebuilding. The implementation was poor (hardcoded values, var global = window),
 but the pattern is exactly what S3+CloudFront needs.

 Recommendation

 The PR should be modified to preserve runtime configuration capability. Two approaches:

 Option A (Recommended): Hybrid approach — keep config.ts validation but source from window.__FLIP_CONFIG__ with
 import.meta.env as fallback

 config.ts reads from:
   1. window.__FLIP_CONFIG__ (runtime, for S3+CloudFront)
   2. import.meta.env (build-time, for Vite dev server)

 This way:
 - Dev mode works unchanged via import.meta.env
 - Production S3+CloudFront serves a /config.js that sets window.__FLIP_CONFIG__
 - The validation logic in config.ts works for both paths
 - The AppConfig type and getConfig() API stay the same

 Option B: Accept build-time config, rebuild per environment

 If the team decides each environment gets its own build, the current PR is fine as-is. But this is generally considered an
 anti-pattern for SPAs.

 Other Review Findings

 Code Quality (from review agents)

 - Overall quality is good — clean separation, proper validation, good test coverage
 - getConfig() is called on every invocation without caching — each call to getConfig() in api.ts, auth.ts, ModelUpload.vue
 re-reads and re-validates. Consider memoizing the result after first successful validation.
 - as string type assertions in getConfig() after validateConfig() are safe but could be avoided with a narrowing return type
 from validateConfig.

 Security

 - XSS safe: main.ts uses textContent (not innerHTML) for error display
 - VITE_AWS_CLIENT_SECRET: All VITE_* vars are embedded in the client bundle by Vite. A Cognito client secret in a frontend
 bundle is a concern — however, Cognito public apps typically don't use client secrets, so this is likely unused. Worth adding
 a comment noting this.
 - No credential leakage: .env.development.example uses placeholder values, test stubs use fake values
 - Old window.js had hardcoded real Cognito IDs (eu-west-2_4QdpwX1GW, 5hcskgab29jpmti0esd655lfsv) — removing this file is a
 security improvement

 Test Coverage

 - config.spec.ts has 156 lines covering required var validation, optional defaults, NaN fallback, and boolean parsing — good
 coverage
 - test/setup.ts stubs required env vars globally so other tests don't break — appropriate approach
 - Gap: No tests for getAuthConfig() in auth.ts to verify it correctly maps config to Amplify format
 - Gap: No integration-level test that main.ts renders the error message when config is invalid

 Type Design

 - AppConfig interface is clean and well-defined
 - awsClientSecret: string | undefined is correct (semantically different from optional ?:)
 - maxReimportCount has no range constraint but the NaN fallback is handled

 Files to Modify

 If proceeding with Option A (hybrid runtime+build-time config):

 ┌────────────────────────────────────────────┬─────────────────────────────────────────────────────────────────────┐
 │                    File                    │                               Change                                │
 ├────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────┤
 │ flip-ui/src/utils/config.ts                │ Read from window.__FLIP_CONFIG__ first, fallback to import.meta.env │
 ├────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────┤
 │ flip-ui/src/env.d.ts                       │ Add __FLIP_CONFIG__ to Window interface                             │
 ├────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────┤
 │ flip-ui/src/utils/__tests__/config.spec.ts │ Add tests for runtime config path                                   │
 ├────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────┤
 │ flip-ui/public/config.js                   │ Template file for runtime config (replaces old window.js)           │
 ├────────────────────────────────────────────┼─────────────────────────────────────────────────────────────────────┤
 │ flip-ui/index.html                         │ Add <script src="/config.js"></script>                              │
 └────────────────────────────────────────────┴─────────────────────────────────────────────────────────────────────┘

 Verification

 1. cd flip-ui && npm run test:unit — all config tests pass
 2. cd flip-ui && npm run build — verify production build succeeds
 3. cd flip-ui && npm run dev — verify dev mode still works with .env.development
 4. Inspect built JS to confirm import.meta.env values are properly substituted