Skip to content

[AgentOps][Code Quality] Better organization for env [1/n]#443

Merged
XinweiHe merged 1 commit intopivot/agentopsfrom
xinwei_env_clean_up_v1
Feb 9, 2026
Merged

[AgentOps][Code Quality] Better organization for env [1/n]#443
XinweiHe merged 1 commit intopivot/agentopsfrom
xinwei_env_clean_up_v1

Conversation

@XinweiHe
Copy link
Collaborator

@XinweiHe XinweiHe commented Feb 9, 2026

Summary

  • Centralize all environment variable access behind validated schemas — Pydantic Settings for Python backend, Zod for Next.js frontend
  • Eliminates scattered os.getenv() / process.env.* reads with hardcoded defaults duplicated at each call site
  • Invalid or missing config now fails fast at startup with clear error messages

Type of Change

  • feat - A new feature
  • fix - A bug fix
  • docs - Documentation only changes
  • style - Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • refactor - A code change that neither fixes a bug nor adds a feature
  • perf - A code change that improves performance
  • test - Adding missing tests or correcting existing tests
  • build - Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
  • ci - Changes to our Cl configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
  • chore - Other changes that don't modify sc or test files
  • revert - Reverts a previous commit
  • security - A security fix or improvement
  • github - Changes to our GitHub configuration files and scripts
  • other (please describe):

Details

Backend (Python)

New backend/shared/config.py using Pydantic Settings:

  • Single Settings class validates all env vars (CLICKHOUSE_*, S3_*, REDIS_URL, INTERNAL_API_SECRET, etc.)
  • All backend modules (rest, worker, db/clickhouse) now import from backend.shared.config instead of reading os.getenv() directly
  • Defaults consolidated in one place

Also cleans up backend/worker/ structure:

  • tasks.pyingest_tasks.py, transformer.pyotel_transform.py (clearer naming)
  • Flattened features/tokens/tokens/
  • Removed empty __init__.py files and stale test file

Frontend (TypeScript)

Two new Zod schema files following the server/client split required by Next.js:

  • frontend/ui/src/env.ts — server-only vars (NEXTAUTH_SECRET, INTERNAL_API_SECRET, SMTP config, Google OAuth)
  • frontend/ui/src/env.client.ts — client-safe NEXT_PUBLIC_* vars with explicit process.env references for Next.js static replacement

Updated 6 files to import from the centralized schemas instead of reading process.env.* directly.

Screenshots / Recordings (if applicable)

N/A — no UI changes, purely internal refactor.

Checklist

  • I have read the CONTRIBUTING.md
  • I have added/updated tests where applicable
  • I have added screenshots or recordings for UI changes (if applicable)
  • I have updated documentation where necessary

@XinweiHe XinweiHe requested a review from a team as a code owner February 9, 2026 07:55
@XinweiHe XinweiHe self-assigned this Feb 9, 2026
@greptile-apps
Copy link

greptile-apps bot commented Feb 9, 2026

Greptile Overview

Greptile Summary

This PR centralizes environment variable access across the Python backend and Next.js frontend.

  • Backend: Introduces backend/shared/config.py (Pydantic Settings) and updates ClickHouse, S3, REST auth dependencies, and Celery worker configuration to read configuration from a single validated settings object rather than scattered os.getenv() calls.
  • Worker reorg: Renames task/transform modules (e.g., tasks.pyingest_tasks.py, transformer.pyotel_transform.py) and flattens token-related modules.
  • Frontend: Adds Zod-based schemas split into env.ts (server-only) and env.client.ts (client-safe NEXT_PUBLIC_*) and updates call sites to consume these schemas.

Main integration point is the shared-secret flow between backend and Next.js internal API routes: the backend now sources traceroot_ui_url and internal_api_secret from Pydantic settings, and the frontend verifies internal requests via Zod-parsed INTERNAL_API_SECRET.

Confidence Score: 3/5

  • This PR is mergeable after fixing internal-secret validation, but currently risks misconfiguration that breaks or weakens internal API auth.
  • Most changes are straightforward refactors to centralize config, but the new env schemas allow INTERNAL_API_SECRET to be empty (backend defaults to "" and frontend allows empty string), which undermines the stated fail-fast behavior and can break internal auth calls or accidentally disable the shared-secret check.
  • backend/shared/config.py, frontend/ui/src/env.ts

Important Files Changed

Filename Overview
backend/rest/routers/deps.py Replaced direct env reads with settings for internal auth calls.
backend/rest/routers/public/traces.py Uses settings for internal API URL/secret and points Celery task import at worker.ingest_tasks.
backend/shared/config.py Introduces Pydantic Settings for ClickHouse/S3/Redis and internal settings; internal_api_secret defaults to empty string which undermines fail-fast + shared-secret auth.
backend/worker/celery_app.py Celery config now uses settings.redis and passes ClickHouse settings into migration env; task autodiscovery adjusted for renamed module.
backend/worker/tests/test_transformer.py Deleted transformer test suite; removes regression coverage for OTEL transform behavior.
frontend/ui/src/env.client.ts Introduced Zod client-safe schema for NEXT_PUBLIC_* vars with explicit process.env references.
frontend/ui/src/env.ts Introduced server env schema; INTERNAL_API_SECRET allows empty string which can disable internal-secret verification.
frontend/ui/src/lib/api/client.ts Switched API base URL to use clientEnv instead of direct process.env reads.
frontend/ui/src/lib/auth-helpers.ts Switched internal secret verification to use centralized server env schema.

Sequence Diagram

sequenceDiagram
  participant Env as Env/.env
  participant Rest as FastAPI (backend/rest)
  participant Settings as backend/shared/config.settings
  participant UI as Next.js (frontend/ui)
  participant Zod as frontend/ui/src/env.ts
  participant Public as REST /public/traces
  participant Deps as REST auth deps
  participant Celery as Celery worker

  Rest->>Env: load_dotenv()
  Rest->>Settings: import settings (Pydantic Settings)
  UI->>Zod: parse(process.env)

  Public->>Settings: read traceroot_ui_url, internal_api_secret
  Public->>UI: POST /api/internal/validate-api-key (X-Internal-Secret)

  Deps->>Settings: read traceroot_ui_url, internal_api_secret
  Deps->>UI: POST /api/internal/validate-project-access (X-Internal-Secret)

  Public->>Celery: process_s3_traces.delay(s3_key, project_id)
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@XinweiHe
Copy link
Collaborator Author

XinweiHe commented Feb 9, 2026

@greptile review

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +68 to +71
# Internal communication (Python <-> Next.js)
traceroot_ui_url: str = "http://localhost:3000"
internal_api_secret: str = ""

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty internal secret

internal_api_secret defaults to "", which means the backend can start up without INTERNAL_API_SECRET and then all internal auth calls that send X-Internal-Secret will fail at runtime (or, if the frontend is also configured with an empty secret, effectively disables the shared-secret check). If the intent is “fail fast at startup”, this should be validated as non-empty (no default) so misconfiguration is caught immediately.

Comment on lines +3 to +7
const serverSchema = z.object({
NEXTAUTH_SECRET: z.string().min(1),
NEXTAUTH_URL: z.string().default("http://localhost:3000"),
INTERNAL_API_SECRET: z.string(),
AUTH_GOOGLE_CLIENT_ID: z.string().default(""),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secret can be empty

INTERNAL_API_SECRET is validated with z.string() (no .min(1)), so an explicitly-set empty string will pass schema validation and verifyInternalSecret() will treat an empty X-Internal-Secret as valid. This makes it easy to accidentally disable internal API authentication; this value should be validated as non-empty.

@XinweiHe XinweiHe merged commit f6ba0f0 into pivot/agentops Feb 9, 2026
4 checks passed
@XinweiHe XinweiHe deleted the xinwei_env_clean_up_v1 branch February 9, 2026 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant