Harden fork CI fidelity: web build fail-fast + test secret isolation#7
Merged
Conversation
A local .dev.vars (gitignored) injects a REAL DISCORD_BOT_TOKEN; CI has none. Code gated on the token (e.g. billing/initiate -> sendBillingOpened) therefore hit Discord for real on a dev machine while CI took the silent no-send branch — tests behaved differently per machine, and the suite made an unintended external POST locally. Delete DISCORD_BOT_TOKEN in the test setup so the baseline is identical everywhere; tests that exercise outbound calls already set their own dummy token + stub fetch. Add a smoke assertion that the token does not leak (reproduces the divergence locally: red before the scrub, green after).
…white-screen bundle) VITE_API_BASE is statically inlined at build time and points the upload page at the fork's own worker. When unset, `vite build` still succeeded and emitted a bundle whose top-level throw fires at load (white screen) — invisible to CI's `pnpm -r build`. Move the gate to build time in vite.config.ts (dev exempt). CI: pass a throwaway VITE_API_BASE to the build step (it only checks the build compiles) and add a regression guard asserting the web build refuses to build without it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent fixes (one commit each) from the Codex review of fork/clean-checkout fidelity. Same theme as #6: make CI/tests reflect what a fork actually experiences.
#4 — scrub external secrets from the test baseline (
ca92f66)A local
.dev.vars(gitignored) injects a realDISCORD_BOT_TOKEN; CI has none. Code gated on the token — notably/admin/billing/initiate→sendBillingOpened(billing.ts:187) oncediscord_billing_channel_idis set earlier in the suite — therefore made a real Discord POST on a dev machine while CI silently took the no-send branch. Tests behaved differently per machine and leaked an outbound call locally.test/apply-migrations.ts:delete env.DISCORD_BOT_TOKENin setup → identical baseline everywhere. Tests that exercise sending already set their own dummy token + stub fetch.test/smoke.test.ts: assertion that the token doesn't leak — reproduces the divergence (red locally before the scrub, green after).#3 — fail the web build when
VITE_API_BASEis unset (baada7e)VITE_API_BASEis statically inlined at build time and aims the upload page at the fork's own worker. When unset,vite buildstill succeeded and emitted a bundle whose top-level throw (web/src/api.ts:2) only fires at load → white screen, invisible to CI'spnpm -r build.web/vite.config.ts: fail the build (not just runtime) whenVITE_API_BASEis missing;devstays exempt..github/workflows/ci.yml: give the build step a throwawayVITE_API_BASE(CI only checks it compiles) + a regression guard step asserting the web build refuses to build without it.Verification
pnpm -r typecheck✅ ·pnpm -r test✅ 180 passing ·pnpm -r build(with dummy) ✅VITE_API_BASE→ fails (exit 1, helpful message); with → succeeds.Neither touches runtime Worker/SPA code → no production deploy needed.