firebase_app_check_tvos: regenerate Pigeon for 0.4.7 and drop the 0.4.6 pin - #26
Conversation
|
Hey @MAUstaoglu, can you please take a look at this follow-up PR and let me know if anything needs to change? Thanks |
MAUstaoglu
left a comment
There was a problem hiding this comment.
Thanks, the Pigeon regen and getTokenResult look right (checked against 0.4.7 from pub.dev). A few things from 0.4.7 are still missing though:
1. App Attest fallback fix not ported. 0.4.7 also fixed appAttestWithDeviceCheckFallback never actually falling back (flutterfire#18568). FirebaseAppCheckPlugin.swift still has the old block, so where App Attest isn't supported (e.g. tvOS simulator) it never switches to DeviceCheck. Please mirror upstream, keeping tvOS 15.0:
import DeviceCheck
...
case "appAttestWithDeviceCheckFallback":
var appAttestProvider: (any AppCheckProvider)?
if #available(iOS 14.0, tvOS 15.0, macOS 14.0, *), DCAppAttestService.shared.isSupported {
appAttestProvider = AppAttestProvider(app: app)
}
delegateProvider = appAttestProvider ?? DeviceCheckProvider(app: app)2. tvos/Classes/Constants.swift still has versionNumber = "0.4.5". Should be "0.4.7".
3. Docs still describe 0.4.5:
README.md: install snippet saysfirebase_app_check: ^0.4.5. Please use">=0.4.7 <0.4.8"like the pubspec, and update the "ported from 0.4.5" header.PORTING_REPORT.md: source/version-alignment sections (lines 5, 60–63) say 0.4.5 is the latest on the train.- The simulator "verified" row in the README was tested on 0.4.5. Either re-verify or mark it as not re-verified for 0.0.2.
4. New example comment is inaccurate. "Exact pins: firebase_core and firebase_core_tvos must move together", but firebase_core_tvos is ^0.0.1 everywhere. Something like # firebase_core is pinned exact; its train selects the matching firebase_core_tvos. would be accurate.
5. Simulator check before merge: run the example and confirm activate() → getToken() / getLimitedUseToken() round-trips with the new 5-arg payload.
Minor, optional: in the package pubspec firebase_core_tvos: ^0.0.3 states the real floor, since 0.4.7 can only resolve 0.0.3 anyway.
|
Hey @MAUstaoglu, made all requested changes. Also verified on the simulator. How do you catch all this? Genuinely asking; I need that Claude skill 😭😭 🥹 |
|
Honestly, there's no magic 😄 I just know how our plugin structure works (how a port relates to its upstream, where versions get pinned, which files tend to go stale), so I point my agent at the things likely to break. Claude does the review and writes up the findings, then I read through them and sign off on what goes out. Thanks for the quick turnaround, and for the before/after simulator check on the App Attest fallback! |
MAUstaoglu
left a comment
There was a problem hiding this comment.
All five points addressed, thanks. Approving.
One optional nit, not blocking: in AppCheckProviderWrapper.configure, the tvOS #else branch of case "recaptcha": only prints. On a first activate() that's fine since delegateProvider starts as nil, but if an app activates with DeviceCheck and later re-activates with reCAPTCHA, the old DeviceCheck provider stays in place. That contradicts the new README row ("provider is left unconfigured"). A one-liner fixes it:
#else
delegateProvider = nil
print("Firebase App Check: reCAPTCHA App Check is not supported on tvOS; use DeviceCheck or App Attest.")
#endifThis behavior is inherited from upstream's macOS branch, so feel free to leave it for a follow-up.
|
@MAUstaoglu, Thanks boss |
Completes the follow-up left open on #25:
firebase_app_check_tvoswas pinned below 0.4.6 because that release moved the platform-interface Pigeon contract. #25 publishedfirebase_core_tvos0.0.3, which unblocks it.What 0.4.6 changed
activaterecaptchaSiteKeygetTokenResultInternalAppCheckTokenResultThe old generated Swift covered neither, so the pin was
">=0.4.5 <0.4.6".Where the sources come from
firebase_app_check0.4.7.FirebaseAppCheckMessages.g.swiftis copied from it; the only delta against upstream is the import guard:getTokenResultinFirebaseAppCheckPlugin.swiftis byte-identical to upstream's.activateand bothconfigure()methods now carryrecaptchaSiteKey, and therecaptchacase passes it toRecaptchaProvider(app:siteKey:)— still inside#if os(iOS), since reCAPTCHA is not a tvOS provider.0.4.7's other Apple-side fix is carried over too:
appAttestWithDeviceCheckFallbackchecksDCAppAttestService.shared.isSupportedrather than the OS version alone, so the provider actually falls back (flutterfire#18568).import DeviceCheckis restored for it and the porter'stvOS 15.0availability kept.Constants.swiftmoves to"0.4.7", and the README /PORTING_REPORT.mdare re-synced off 0.4.5.Why a closed window, not a caret
firebase_app_check: ">=0.4.7 <0.4.8", and the example pins exact.This is the package that caused the rule: on #18 a
^0.4.5silently admitted 0.4.6, which moved the Pigeon contract and produced aMissingPluginExceptionplus a silently-dropped argument.^0.4.7would admit 0.4.8 the same way.0.4.8 has since shipped. Its Apple sources are byte-identical to 0.4.7 apart from
versionNumber, it declares the samefirebase_app_check_platform_interface: ^0.4.2+1, and its only substantive change is bumping the Firebase iOS SDK to 12.19.0 — so the contract did not move and the generated Swift here already covers it. Widening to">=0.4.7 <0.4.9"is therefore defensible (a closed window over a diffed version, not a caret over an unverified one), but left at<0.4.8unless you prefer otherwise.firebase_core_tvosis floored at^0.0.3here rather than the fleet's^0.0.1. 0.4.7 requiresfirebase_core_platform_interface ^8.1.1: 0.0.2 excludes itself by declaring pi>=7.1.0 <8.1.0, but 0.0.1 declares no pi constraint at all, so the solver can legally pick it, and its native Pigeon expects the pre-8.1FirebaseOptionspayload — a crash at launch rather than a resolution error. The floor makes that a solver error instead. It costs nothing: this package cannot serve the 4.11.x train anyway.Review #25, point 4
Six example pubspecs carried comments describing the 15-field payload as matching
firebase_core_tvos's native Pigeon — true for 0.0.2, misleading once 0.0.3 is published. Each is replaced with two lines naming whichfirebase_core_tvosthefirebase_corepin actually selects. Note^0.0.1was never an exact pin: in pub semantics it is>=0.0.1 <0.1.0, so it floats across 0.0.1–0.0.3 and thefirebase_corepin is what resolves it. Versions are unchanged: every PORTING_REPORT names a specific upstream, so the exact pins are deliberate.Only
firebase_app_check_tvoshas changes outsideexample/, so no other package needs a release.Verified
Payload arity, both ends:
activate(interface 0.4.2+1)activategetTokenResultAll seven touched examples resolve and analyse clean. They do not all land on the same
firebase_core_tvos: this package's example resolves 0.0.3, while the other six pinfirebase_core: 4.11.0and 0.0.3 requiresfirebase_core ^4.13.0, so they resolve 0.0.2 — correct for that train, and what the new comments say.Simulator pass — Apple TV 4K (3rd generation), tvOS 26.5, native
Firebase/AppCheck 12.15.0, live Firebase project:activate(Debug)getToken…/exchangeDebugToken, HTTP 403 (debug token not registered)getLimitedUseTokenactivate(DeviceCheck)getToken→ "DeviceCheckProvider is not supported…"activate(AppAttestFallback)getToken→ "DeviceCheckProvider is not supported…"All three
activate()calls crossed the 5-argument payload with no codec error, no arity error and noMissingPluginException. Both token calls reached the App Check backend with the right project and app ID and returned typedFirebaseExceptions to Dart; the 403 is a console-registration gap, not a plugin defect.The last row is the fallback fix working: the simulator reports tvOS 26.5, so the old
#available(tvOS 15.0)check passed and selected App Attest — a control build with the pre-fix block reports "AppAttestProvider is not supported…" on the same call. The example gained anactivate (Debug)and agetLimitedUseTokenbutton so the sequence is reproducible by hand.Release order
firebase_core_tvos0.0.3 is already on pub.dev, so there is no blocking chain: this can publish as soon as it is merged.