fix(server): drop CODEGEN_SERVICE_URL and source SENTRY_DSN from env file#1080
Conversation
The release-server workflow's build:server step failed because the CODEGEN_SERVICE_URL and SENTRY_DSN production values aren't configured as CI secrets, so validateProductionEnv() rejected the empty strings. - Remove CODEGEN_SERVICE_URL end to end: it was inlined into server config and threaded into HttpServerConfig but never actually read, and the EXTERNAL_URLS.CODEGEN_SERVICE constant had no consumers (dead code). - Bake the real SENTRY_DSN into .env.production.example (a Sentry DSN is a public client identifier already inlined into the shipped binary, mirroring BROWSEROS_CONFIG_URL) and drop the now-empty SENTRY_DSN line from the workflow env block. Otherwise process.env's empty string shadows the file value via 'process.env[key] ?? fileEnv[key]'. Verified: typecheck, biome, build smoke test (2 pass), and go build all green.
|
Greptile (@greptileai) review |
|
Claude (@claude) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 2m 47s —— View job Code Review
SummaryThis is a clean, well-reasoned fix. The root cause analysis and the Correctness
The const value = process.env[name] ?? fileEnv[name]When the workflow sets The build path is correct. The workflow calls Observations1. 2. Dogfood 3. README 4. VerdictLGTM with minor suggestions. The core fix is correct and complete. Items 1–4 above are observations and minor polish, not blockers. The test plan covers the critical paths. |
✅ Tests passed — 1059/1064
|
Greptile SummaryThis PR fixes a failing CI build by removing the dead
Confidence Score: 4/5Safe to merge for CI builds; dogfood production builds will regress until the Go default is patched. The CI workflow fix is correct and the dead-code removal is thorough. The one gap is DefaultProductionEnv() in the Go dogfood tool, which still hard-codes SENTRY_DSN as empty. Because the DSN is now a public constant stored only in .env.production.example, any dogfood user who triggers a production build will hit validateProductionEnv failing on an empty SENTRY_DSN — a regression introduced by this PR that requires a manual workaround until the default is updated. packages/browseros-agent/tools/dogfood/config/config.go — DefaultProductionEnv still returns an empty SENTRY_DSN. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["CI: release-server workflow"] -->|copies| B[".env.production.example → .env.production"]
A -->|no longer injects| C["SENTRY_DSN and CODEGEN_SERVICE_URL removed from env block"]
B -->|fileEnv| D["buildInlineEnv: process.env key ?? fileEnv key"]
E["process.env: POSTHOG_API_KEY from CI secret"] --> D
D -->|inlined at build time| F["Server Binary: SENTRY_DSN, POSTHOG_API_KEY, BROWSEROS_CONFIG_URL"]
G["Dogfood tool"] -->|generates env with SENTRY_DSN empty| H["buildInlineEnv receives empty string"]
H -->|empty string passes ?? check| I["validateProductionEnv FAILS: SENTRY_DSN required"]
style I fill:#ffcccc,stroke:#cc0000
style F fill:#ccffcc,stroke:#007700
|
Greptile SummaryThis PR fixes a broken
Confidence Score: 4/5Safe to merge — the CI fix is correct and the dead-code removal is thorough across all layers. The core logic change (removing the empty workflow env lines so the file value isn't shadowed by empty-string injection) is sound and well-explained. Dead-code removal of CODEGEN_SERVICE_URL is consistent across TS, Go, env files, and the workflow. The only gaps are documentation: the README default column is now stale for SENTRY_DSN, and the dogfood DefaultProductionEnv still defaults SENTRY_DSN to an empty string even though the value is now publicly committed — meaning new dogfood users attempting a local production build would still hit a validation error without knowing where to find the DSN. packages/browseros-agent/tools/dogfood/config/config.go and packages/browseros-agent/README.md — both have minor documentation gaps created by the new committed DSN value. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CI: release-server workflow] --> B[cp .env.production.example\n→ .env.production]
B --> C[Build step\nenv: BROWSEROS_CONFIG_URL, POSTHOG_API_KEY]
C --> D[loadBuildConfig]
D --> E[loadProdEnv\nreads .env.production file]
D --> F[buildInlineEnv]
E --> F
F --> G{"process.env[key]\n?? fileEnv[key]"}
G -->|SENTRY_DSN: undefined in process.env| H[fileEnv value\nhttps://08d37...]
G -->|POSTHOG_API_KEY: set from secret| I[secret value]
G -->|BROWSEROS_CONFIG_URL: set from workflow| J[workflow var value]
H --> K[validateProductionEnv ✓]
I --> K
J --> K
K --> L[bun build --env inline\nvalues baked into binary]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/browseros-agent/README.md:122
The `SENTRY_DSN` default column now shows `-` but the value is pre-populated in `.env.production.example`. Updating the table keeps the docs in sync with the template and signals to users they don't need to supply their own DSN.
```suggestion
| `SENTRY_DSN` | (set in `.env.production.example`) | Sentry DSN baked into prod binary |
```
### Issue 2 of 2
packages/browseros-agent/tools/dogfood/config/config.go:199-203
Since `SENTRY_DSN` is now a public value committed in `.env.production.example`, dogfood's `DefaultProductionEnv` could carry it too. Without this, new dogfood users who attempt a local production build will hit a `validateProductionEnv` failure on `SENTRY_DSN` unless they manually add it to their config YAML.
```suggestion
return ProductionEnv{
Server: map[string]string{
"BROWSEROS_CONFIG_URL": "https://llm.browseros.com/api/browseros-server/config",
"POSTHOG_API_KEY": "",
"SENTRY_DSN": "https://08d37194d0cfaed69687c3803216daa4@o4510545525932032.ingest.us.sentry.io/4510545527111680",
```
Reviews (2): Last reviewed commit: "fix(server): drop CODEGEN_SERVICE_URL, s..." | Re-trigger Greptile |
Keeps dogfood's server prod-env map aligned with .env.production.example, which now ships the public Sentry DSN. SENTRY_DSN is a non-secret client identifier (like BROWSEROS_CONFIG_URL, already hardcoded here), so it belongs with the non-secret defaults; POSTHOG_API_KEY and R2_* stay empty as genuine secrets supplied via user config.
|
Thanks Greptile (@greptileai) and Claude (@claude) — addressed below. ✅ dogfood
|
|
Claude finished Nikhil (@shadowfax92)'s task in 44s —— View job Claude finished Nikhil (@shadowfax92)'s task —— View job Acknowledged
Dogfood
|
|
Fair corrections on both counts — thanks for the clear reasoning. On the regression framing: you're right, I misstated that. On the README "Default" column: the distinction between code-level fallbacks vs. template-sourced values is sound, and keeping LGTM — the approach is well-reasoned and the test plan covers the key scenarios. |
Reverts 7244a05. Dogfood never runs the validating build:server (it builds the agent in dev mode), so it doesn't need the DSN; no reason to bake the value into Go source.
|
Correction on the dogfood item: reverted in No env value should be hardcoded into the Go source here. As noted above, dogfood never runs the validating Net: both Greptile suggestions declined; the core CI fix (commit |
1 participant
Summary
release-serverworkflow'sbuild:serverstep was failing CI —validateProductionEnv()requiresCODEGEN_SERVICE_URLandSENTRY_DSN, but neither is configured as a CI secret, so they resolved to empty strings and the build aborted (Production build requires variables: CODEGEN_SERVICE_URL, SENTRY_DSN).CODEGEN_SERVICE_URLend to end — it was inlined into server config and threaded intoHttpServerConfig, but never actually read; theEXTERNAL_URLS.CODEGEN_SERVICEconstant also had zero consumers (dead code).SENTRY_DSNfrom the committed env template instead of an (unconfigured) CI secret.Design
A Sentry DSN is a public client identifier — it's already inlined into every shipped binary, exactly like
BROWSEROS_CONFIG_URL, which ships its real value in.env.production.example. So the DSN now lives in that template, which CI copies to.env.productionbefore building. The subtle part:buildInlineEnvresolves values asprocess.env[key] ?? fileEnv[key], and'' ?? x === '', so an emptySENTRY_DSNinjected by the workflowenv:block would shadow the file value. The fix therefore also drops the now-emptySENTRY_DSNandCODEGEN_SERVICE_URLlines from the workflow so the file value actually takes effect.POSTHOG_API_KEYis untouched — its secret is configured and still flows from the workflow env.Test plan
bun run typecheck— all packages passbunx biome checkon changed files — clean (exit 0)bun test apps/server/tests/build.test.ts— 2 pass (real compile +--version, plus--ciarchive build)go build ./...intools/dogfood— passesloadBuildConfigbefore the change; confirmed the fixed scenario (SENTRY from file, no CODEGEN anywhere) loads cleanly after.