fix(mobile): local android:apk builds with test ads (real ads stay in android-release.yml) - #332
Merged
Merged
Conversation
The local APK and device-install scripts forced EAS_BUILD_PROFILE=production and threw when prod AdMob IDs were absent, blocking all local QA builds. Rename production-admob-env to test-build-admob-env and switch both scripts to resolveTestBuildAdMobEnv, which forces EXPO_PUBLIC_ADMOB_USE_TEST_IDS=true and leaves the build profile unset so app.config.js defaults to test ads and skips the prod assert. Real-ad releases are unaffected: they ship only through the self-contained android-release.yml CI workflow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review PR 332 — APPROVE
This PR fixes the local android:apk regression where production-admob-env.js threw without live AdMob IDs. The fix is correct: resolveTestBuildAdMobEnv sets EXPO_PUBLIC_ADMOB_USE_TEST_IDS=true, never throws, never forces EAS_BUILD_PROFILE=production. The CI release path (android-release.yml) is entirely self-contained and unaffected — it injects real IDs at the job level and guards via assert-production-admob-config.js and the AndroidManifest sample-ID scan.
FINDINGS: None (Critical 0, High 0, Medium 0, Low/Info 0)
WHAT IS GOOD: Root-cause fix not a workaround. CI release path airtight and untouched. Dead code fully cleaned up (zero references to production-admob-env remain). Three focused unit tests cover key behavioral guarantees. SKILL.md updated. New module is 8 lines — nothing over-abstracted.
|
thomasluizon
added a commit
that referenced
this pull request
Jun 27, 2026
) After #332 unblocked the AdMob guard, npm run android:apk compiled the full app (~18 min) then failed at the Sentry source-map upload: 'Auth token is required' — that step needs the CI-only SENTRY_AUTH_TOKEN. Set SENTRY_DISABLE_AUTO_UPLOAD=true in the two local build scripts; sentry.gradle.kts guards the upload with onlyIf { shouldSentryAutoUploadGeneral() } == (getenv('SENTRY_DISABLE_AUTO_UPLOAD') != 'true'), so it's skipped at gradle time. CI release sets the token + omits this env, so prod source-map uploads are untouched. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
added a commit
that referenced
this pull request
Aug 19, 2026
) The canvas run is complete. The screens project holds 20 documents and 21 pages, all new canon, and no document survives from the first run. Screens built: Hoje, habit create, habit detail, Calendario, Progresso, the Astra conversation, onboarding, auth, Perfil, notifications, the Pro pitch, the subscription manage view, the surviving celebration, the error and static surfaces, offline, search, step up, the overlay primitives, Wrapped, the four static screens, and the Android widget. Three design system rounds ran alongside the screens, each from gaps a build reported rather than from a guess: - D the four defects found on the first Hoje: icon centring, emoji used as iconography, frozen and skip as habit statuses, and a hand drawn mark - D2 DayStrip, Checkbox, a shell header slot, TimeField, StatTile states, Menu - D3 BlockFrame's control, proposed and irreversible rows plus a risk slot, Composer's busy state and its discriminated atLimit, and an authorable conversation slot Two of those are enforced by contract rather than by prose, because the same bug came back twice under different disguises. DayStrip discriminates on scope, so passing frozen to a habit strip is a type error. Composer discriminates on state, so atLimit cannot render without its message and no other state can pass one. Six open questions were put to Thomas and answered: a rejected preview collapses to one line, step up is a hand off, a partially failed bulk create keeps what it created, the day arc is the exact fraction, Calendario pages on an empty account, and the month rate has a definition. DESIGN.md gains that definition, because Calendario computed it and correctly flagged that no endpoint states the window, which means the next surface would compute a different number from the same data. Four API tickets were filed from what the screens reported they needed: #331 the streak repair endpoint, #332 the achievements payload, #333 the Astra metrics schema, #334 the notification urls. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Local Android builds use Google TEST AdMob units
npm run android:apk(theandroid-generateskill) and the local device-install script were blocked: they forcedEAS_BUILD_PROFILE=productionand threw without the production AdMob IDs — a requirement that should only apply to the real CI release.Change
production-admob-env.js→test-build-admob-env.js;resolveTestBuildAdMobEnvsetsEXPO_PUBLIC_ADMOB_USE_TEST_IDS=true, never throws, and doesn't force the production profile — soapp.config.jsuses test ads and skips the prod assert.android-release-apk.js+android-release-device.jsuse it (with a log line noting test ads).SKILL.mddocuments local = test ads.Real-ad release is untouched
.github/workflows/android-release.ymlis self-contained — it injects the real AdMob IDs at job level and guards viaassert-production-admob-config.js(a different module) + a generated-manifest sample-ID check. It never used these local scripts.Validation
node --checkall scripts; new unit test 3/3; lint clean for the changed files.🤖 Generated with Claude Code