fix(recaptcha): address review items for requestHooks bridging

[ncooke3]

• Add AppCheckRecaptchaProvider as dependency to AppCheckCoreUnitObjC and add an Objective-C bridging test constructing GACRecaptchaProvider with request hooks.
• Define invalidRequestHook (3002) message code in AppCheckCoreErrors and use it when logging rejected hooks.
• Add CHANGELOG entry for GACRecaptchaProvider requestHooks bridging fix.
• Format doc-comment parameters across AppCheckRecaptchaProvider convenience initializers.

Addresses #126 (review)

[ncooke3]

Merging. Can resolve any feedback in additional PRs/commits.

[paulb777]

For the record, these are just nits, and may be addressed in a cleanup task later:

Review: #127 — "fix(recaptcha): address review items for requestHooks bridging"

PR: google/app-check#127 · pb-swift@e5217113 ← nc.fix.recaptcha-review-items
Author: ncooke3 · 5 commits, 7 files, +38 / −10 · mergeable_state: clean, 16/16 checks green
Addresses: my review of #126 — findings F1, F2, F4, F5.

Review history:

• Pass 1 at 6de25bb3 — two compile blockers (B1, B2), one open question (Q1).
• Pass 2 at eeeea538 — blockers unchanged; traced both to a single suggestion-apply, and corrected my attribution.
• Current: pass 3 at 3f95be91 — both blockers resolved, CI fully green.

---

Verdict

Approve, with one nit worth a follow-up line and one open question that predates this PR's mechanics.

3f95be91 ("add missing pound symbols to preprocessor directives") restores the directive:

-if (TARGET_OS_IOS || TARGET_OS_VISION) && !TARGET_OS_MACCATALYST
+#if SWIFT_PACKAGE && (TARGET_OS_IOS || TARGET_OS_VISION) && !TARGET_OS_MACCATALYST

That single line clears B1 (bare if at file scope plus an orphaned #endif) and B2 (@import of a module CocoaPods does not build) together, exactly as expected — both were one regression from 6de25bb3.

CI now proves it rather than predicting it

Job
pod_lib_lint (ios, macos-15) / (ios, macos-26)	✅ — B2 confirmed resolved on the leg that would have failed
pod_lib_lint macos / tvos / watchos, both runners	✅
swift-build-run iOS / tvOS / macOS / catalyst	✅ — B1 confirmed on every platform
catalyst, warnings-as-errors	✅

The warnings-as-errors pass settles something I had only argued from deployment targets in pass 2: removing if (@available(iOS 15.0, visionOS 1.0, *)) in eeeea538 produces no -Wunguarded-availability diagnostic. That is now measured, not inferred.

---

🟡 New nit — SWIFT_PACKAGE was applied to the test as well as the import

3f95be91 added the guard to both sites. 1731eb32 had it on only one, and that was the better configuration:

	1731eb32	3f95be91 (head)
@import AppCheckRecaptchaProvider;	#if SWIFT_PACKAGE && …	#if SWIFT_PACKAGE && …
testRecaptchaProviderRequestHooksBridging	#if (TARGET_OS_IOS || TARGET_OS_VISION) && !TARGET_OS_MACCATALYST	#if SWIFT_PACKAGE && …

Only the import needs SWIFT_PACKAGE, because only SwiftPM has a separate module. The test compiles fine under CocoaPods: AppCheckRecaptchaProvider.swift:15 puts only import AppCheckCore behind #if SWIFT_PACKAGE — the @objc(GACRecaptchaProvider) class itself is unconditional, and the podspec compiles it into the AppCheckCore module on iOS, so @import AppCheckCore; already reaches it.

Net effect: the reCAPTCHA bridging test no longer exists on the CocoaPods leg. The green pod_lib_lint (ios) therefore proves the file builds there, not that the test ran there — and CocoaPods is the build system where module and access-level differences have actually bitten this branch before (af0bc656). Dropping SWIFT_PACKAGE && from the test guard alone recovers that coverage.

Not worth blocking on; worth a one-line follow-up.

---

🟡 Q1 — still open: can the new test fail against the bug?

Unchanged through all three passes, and now the only substantive item left.

RecaptchaEnterpriseSDKLoader resolves its classes with NSClassFromString("RecaptchaEnterprise.RCARecaptcha"), and RecaptchaEnterprise is not linked into the test target — the package depends on RecaptchaInterop, the protocol shim, not the SDK. So the initializer takes its first branch:

guard let sdk = RecaptchaEnterpriseSDKLoader(customAction: actionName) else {
  return nil            // ← before `requestHooks` is ever read
}

The test detects a reverted signature only if the typed-array trap fires in the @objc thunk during argument bridging, rather than on first element access. I argued thunk-time in #126: the thunk must produce Array<Element> where Element is a function type, which is neither a class, an ObjC existential, nor _ObjectiveCBridgeable, so the conversion cannot be verbatim and must be eager. The author's doc comment on testRequestHooksBridging states the opposite — "bridging is lazy, so the element cast is forced on first access." Both cannot hold, and a green CI run says nothing either way, because the bug is not present to be caught.

One experiment settles it. Revert the reCAPTCHA parameter to [@convention(block) (NSMutableURLRequest) -> Void]? locally and run the test:

• Crashes → the test is sound, one of the two comments is wrong, and the thunk-time behaviour belongs in this test's doc comment.
• Passes → the test is inert and needs to reach the array, via a stubbed loader or by exercising the provider's forwarding into _GACAppCheckAPIService.

---

Carried nits

1. The test has no assertion and no comment saying why. Its entire signal is "does not crash." In a PR that exists because an earlier test could not fail for its own regression, an assertion-free body with no explanation is the shape of thing a later cleanup deletes. Resolving Q1 gives you exactly the sentence to put there.
2. Changelog comma splice — "…now accepts NSArray<id> * / [Any]? fixing an @objc argument bridging trap…" wants a comma before fixing.

---

Summary

Items addressed (F1, F2, F4, F5)	✅ All four
B1 — missing #	✅ Fixed in 3f95be91
B2 — CocoaPods module	✅ Fixed in 3f95be91, confirmed by pod_lib_lint (ios)
@available removal	✅ Safe, confirmed by warnings-as-errors
Test runs under CocoaPods	🟡 No — over-gated on SWIFT_PACKAGE
New test is load-bearing	❓ Q1 — unresolved
Message code work	✅ Exceeded the ask

Mergeable as-is. Q1 is the one thing I would want answered before treating this test as protection rather than decoration — and it is a five-minute local experiment, not a code change.