Keep the price lookup alive when the connect race is lost, and bump to 212 - #5
Merged
StaticHumStudio merged 4 commits intoAug 22, 2026
Merged
Conversation
A free user could open the unlock screen and find a disabled button spinning on "Checking price" forever, with no in-app way to clear it. Restore did not help, because Restore re-asks what you own and not what things cost. On the device tonight this reproduced on 10 of 10 cold starts. WHAT BREAKS. connect() calls startConnection while enableAutoServiceReconnection already has a connection in flight. The library rejects the second caller with DEVELOPER_ERROR and says so plainly: "Client is already in the process of connecting to billing service." Our setup listener treats any non-OK code as "Play is unavailable" and returns, so the refreshPurchases + loadProductDetails pair behind it never runs. Play is fine throughout. Finsky resolves the billing account normally in the same log window, and the connection the other caller opened serves queries happily. The entitlement restore proved it: it worked on every one of those same failed launches, including after a full pm clear. WHY THAT ASYMMETRY IS THE REAL BUG. refreshPurchases had three callers, so it healed itself and tested clean. loadProductDetails had exactly one, sitting inside the branch the race kills, so it failed silently and only for people who had not bought yet. A tester who owns the product cannot see this failure at all, which is why a passing purchase pass sat right next to it. So the fix is a second trigger, not a cleverer connect. Suppressing the double connect would mean timing our call against the library's internal reconnection schedule, which we do not control and which the next library update is free to change. Instead loadProductDetails now runs from the same foreground hook that already rescues refreshPurchases: a path observed working 10 times tonight. It returns immediately once a price is in hand, and a failed lookup leaves the product null so the next foreground retries. Comments at both sites record what was measured, so nobody restores the old single-trigger shape believing the setup callback is dependable. NOT UNIT TESTED, deliberately. BillingManager builds its BillingClient in its own constructor and no test in this repo touches either class. The honest options were a refactor I am not doing unattended, or a tautological policy object invented to look like coverage. Neither earns its keep. 416 existing tests still pass and the behavioural proof is named in the receipt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… to 212 Review finding (P2) on the previous commit, and it was right. The early return on a cached ProductDetails meant that once a price landed it was never asked for again, where the code before this branch re-queried on every successful reconnect. That trades one bug for a quieter one. ProductDetails carries the offer token the purchase flow actually spends, prices vary by region, and this file already notes the ladder moves with each feature drop. An audiobook player process can live for days. A price pinned on first launch is a price that can be wrong by the time somebody presses the button, and the failure lands on the same button this branch exists to unbreak. Dropping the guard also makes the thing symmetric, which was the whole argument. refreshPurchases re-asks on every foreground. Now so does the price. The mutex still skips overlapping calls rather than queueing them, and a failed lookup leaves a previously good product in place, so one bad query cannot cost a working price. versionCode 212 rides along. 212 is the build that carries both the claim-prompt removal and this fix, and it is the one a Play reviewer should open. Folded in here rather than split into its own PR so the build needs one review cycle instead of two. 416 tests, 0 failures. assembleDebug clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review finding (P1) on the previous commit, introduced by that commit. Dropping the cache guard was right, but the guard had been doing two jobs and only one of them got credited. It also stopped the unconditional assignment below from ever running twice, which hid the fact that the assignment overwrites on every path. An OK response is not a promise that our product came back with it. Play reports unfetched products separately from the overall response code, so a successful query returning an empty or non-matching list is routine. That null was being written straight over a good price, leaving priceLookupSettled true with nothing to show, which renders the button as "Unavailable" and disables it. That is the same dead purchase surface this branch exists to remove, reached through a different door. Now the assignment happens only on a hit. Keeping the last known price carries the opposite risk, and a much smaller one: if the product really were deactivated, the user taps buy and Play declines. A recoverable error beats a button nobody can press. A device that never had a price still keeps null and still shows the unavailable copy, because there that is the honest answer. 416 tests, 0 failures. assembleDebug clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review finding, first half. oneTimePurchaseOfferDetails is nullable in the Billing API, and it is both the price the unlock screen displays and the object launchPurchase hands to Play. A product that matches the id but carries no offer prices as null, which disables the button exactly as thoroughly as having no product at all. Worse, it looked like a cache hit, so it would sit there and block the next lookup from replacing it with a good one. Matching the id is not enough. Match the id AND a usable offer, or treat the response as a miss and keep whatever last worked. Second half of that finding DECLINED for this branch, deliberately: re-querying product details inside the purchase path before launching the billing flow. Reasoning, so the next person does not have to guess. The staleness window is now at most one foreground, because this branch already makes the price re-query on every foreground rather than caching it. For the retained object to be genuinely unusable, Play has to have invalidated it inside that window. If it did, launchBillingFlow returns an error and the next foreground replaces the object. That is a recoverable failure. The proposed fix puts a network round trip in front of the single most important button in the app, adding latency and a brand new failure mode to the one interaction that must not break, and there is no device on this machine tonight to watch it behave. Trading a rare recoverable error for an untested change to the purchase interaction is the wrong direction at 1am. Filed as follow-up rather than merged blind. 416 tests, 0 failures. assembleDebug clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
StaticHumStudio
deleted the
fix/price-lookup-survives-connect-collision
branch
August 22, 2026 05:01
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.
What was broken
A free user could open the unlock screen and find the purchase button disabled, spinning on "Checking price", forever. No in-app action cleared it. Restore did not help, because Restore re-asks what you own and not what things cost. The only escape was force-closing the app.
connect()callsstartConnectionwhileenableAutoServiceReconnection()already has a connection in flight. The library rejects the second caller withDEVELOPER_ERRORand says so in the log:Our setup listener treated any non-OK code as "Play is unavailable" and returned, so the
refreshPurchases()+loadProductDetails()pair behind it never ran.Measured at 10 of 10 cold starts on the SM-S948U against Play-installed 2.1.0 (211). Not intermittent. Force-stop and relaunch, every time, including on freshly
pm cleared data.Play is fine throughout. Finsky resolves the billing account normally in the same log window, and the entitlement restore succeeded on every one of those same failed launches, including after a full data wipe. That is what proves the underlying connection is live and serving queries.
Why the asymmetry was the real bug
refreshPurchases()had three callers, so it healed itself and tested clean.loadProductDetails()had exactly one, sitting inside the branch the race kills. So it failed silently, and only for people who had not bought yet.A tester who owns the product cannot see this failure at all. That is why it sat undisturbed next to a purchase pass that passed.
The fix
A second trigger, not a cleverer connect. Suppressing the double connect would mean timing our call against the library's internal reconnection schedule, which we do not control and which the next library update is free to change.
loadProductDetails()now runs from the same foreground hook that already rescuesrefreshPurchases(), a path observed working ten times. It re-queries rather than caching the first answer, and it refuses to let an empty or offerless response erase a price that already worked.versionCode212 rides along, folded in rather than split so the build needs one review cycle instead of two.Review
Four rounds, codex
gpt-5.6-lunaat xhigh. Final round: P0, P1 and P2 all empty, "the branch is clear." Rounds 1 through 3 each raised one finding and each was fixed, which is recorded commit by commit.One item declined, on the record: re-querying product details inside the purchase path before launching the billing flow. The staleness window is at most one foreground now that the price re-queries every foreground, the failure mode is a recoverable
launchBillingFlowerror, and the proposed change puts a network round trip in front of the single most important button in the app. Untested, unattended, at 1am, that is the wrong trade. Worth revisiting deliberately.Verification, including what is missing
assembleDebugclean.bundleReleasesigned,jar verified.verifyWorkManagerKeepRulesReleasepassed.BillingManagerbuilds its ownBillingClientand no test in this repo touches either class. The options were a refactor not worth doing unattended, or a tautological policy object invented to look like coverage. Neither earns its keep, one week after deletingPurchaseGatePolicyfor being machinery.Device check owed after upload: cold-start 212 and confirm the collision no longer prevents the price from loading. The clean proof of the user-visible half needs a second license-tester account that does not own the unlock, installed fresh from Play, because Play pins billing to the installing account and switching accounts in the Play Store app does not move it. Without that account nobody can ever see the screen a real new user sees.
Evidence:
~/nine-lives-evidence/2026-08-22-purchase-pass/SUMMARY.md🤖 Generated with Claude Code