Architecture refactor: encapsulation + testability across Core / TapToPay

[thejohnnybot]

Summary

• Documents and resolves an end-to-end architecture audit of the SDK against authoritative Swift library guidance (Apple's API Design Guidelines, SE-0386 package access, WWDC19 #410, swift-nio / swift-log / swift-argument-parser conventions). Findings + decision-ready proposals live in Documentation/ArchitectureAssessment-2026-05-06.md; the bite-sized implementation plan lives in Documentation/Plans/2026-05-06-architecture-refactor.md.
• Implements every actionable finding (17 of 19; F8 and F17 explicitly deferred in the assessment, F18 skipped because PaymentCardReader.isSupported is load-bearing) and all five proposals, with xcodebuild test green after every commit.
• Establishes a single shared session backbone (PayabliSession), a transport seam (PayabliTransport + AuthenticatedTransport decorator) so bearer-header injection and 401-refresh-and-retry happen in one place, and a real PayabliSDKTestUtils library product (modeled on NIOTestUtils / swift-log's InMemoryLogging) so host apps and future modules can depend on the SDK's stub fixtures instead of re-implementing them.

Highlights of the structural change

• Core gains every cross-module primitive: RetryPolicy, generic EventMulticaster<Event>, PayabliTransport protocol, AuthenticatedTransport decorator (internal), PayabliSession, mapPayabliHTTPError free function, and PayabliAuth.tokenChanges() AsyncStream.
• PayabliConfig is now a value type. PayabliSession is the identity-bearing object that wires one PayabliAuth + one PayabliService for sharing across components — when PayIn re-lands from develop it constructs from the same session.
• No more open-coded 401 retry in TapToPay. tryUpdate calls session.transport.perform(...) and the decorator handles the refresh-and-retry once; double-401 throws .tokenExpired. AppAttestService follows the same path (no manual bearer injection).
• Public surface tightened: SessionManager, SessionTierValidator, FiservCardReader.Credentials, AppAttestService hardware-id providers, PayabliService.makeDefaultSession, AppAttestService+Defaults providers, AuthenticatedTransport, KeychainStorage non-protocol methods, and PayabliEnvironment.local (gated behind #if DEBUG) all demoted from public to internal (or DEBUG-only).
• PayabliSDKTestUtils shipped as a real library product. Six fixtures (StubURLProtocol, InMemorySecureStorage, MockAppAttestor, MockTapToPayProvider, MockDeviceAttestationService, InMemoryTelemetryTransport) consolidated into Sources/PayabliSDKTestUtils/, with a smoke-test bundle. All mock state is now NSLock-synchronized so the @unchecked Sendable conformances are honest.
• @testable import hygiene: 12 of 23 test files no longer need @testable after public-surface tightening; remaining 11 keep it for documented internal-access reasons.
• Dead code removed: TapToPayProviderFactory (no production caller), the duplicated service/auth ivars on PayabliTTP, the Locked/UncheckedSendableBox types extracted from the facade into a dedicated file.
• Module versions now read from Bundle(for:).infoDictionary["CFBundleShortVersionString"] with "0.0.0" fallback — no more "1.0.0" literal drift.

Resolution table

Full mapping of every finding/proposal to its commit SHA(s) is at the bottom of Documentation/ArchitectureAssessment-2026-05-06.md under "Resolution status". Of the three follow-ups originally noted there, two are closed in this PR (AppAttestService+Requests transport-seam migration; AuthenticatedTransport demoted to internal). The remaining follow-up — PayabliSession.auth/.service → package access — stays as a separate PR since it requires verifying package works across test bundles.

API surface changes

The two consumer-facing inits on PayabliTTP (the accessToken:-based convenience init and the config:-based init) keep their signatures verbatim. The @objc convenience init chain is intact. The config: init is now a convenience init that builds a PayabliSession and delegates; the new session: init is the designated init.

Three accidentally-public methods on KeychainStorage (data(forKey:), set(_ data:forKey:), removeAll()) are demoted to internal. They were never documented in the README, never part of the SecureStorage protocol, and existed only because the original struct didn't tighten its access modifiers. The audit (Finding 19e) recommended trimming them. Anyone reaching directly into KeychainStorage for raw Data storage rather than through the SecureStorage protocol would need to switch to the protocol. No documented consumer is affected.

Test plan

• CI: xcodebuild test -scheme PayabliSDK-Package -destination 'platform=iOS Simulator,name=iPhone 17 Pro,OS=26.4.1' — passes locally on every commit; verify clean run on the GitHub Actions runner.
• Smoke: build a host app against this branch and confirm import PayabliSDKTapToPay plus the existing PayabliTTP(accessToken:tokenProvider:entryPoint:appId:environment:) init compile unchanged.
• Smoke: inspect Documentation/ArchitectureAssessment-2026-05-06.md resolution table and confirm each row's commit SHA exists on this branch (git show <sha>).
• Spot check: read Sources/PayabliSDKCore/Public/PayabliSession.swift and Sources/PayabliSDKCore/Networking/AuthenticatedTransport.swift end-to-end — these are the new architectural seams.
• Decide: PayabliSession.{auth, service} visibility (only remaining open follow-up).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR implements a cross-module architecture refactor to improve encapsulation and testability across PayabliSDKCore and PayabliSDKTapToPay, introducing a shared PayabliSession backbone and a transport seam (PayabliTransport + AuthenticatedTransport) while consolidating reusable test fixtures into a shipped PayabliSDKTestUtils product.

Changes:

• Introduces PayabliSession (Core) and a transport decorator to centralize bearer-header injection and 401 refresh-and-retry behavior.
• Moves/generalizes shared primitives into Core (e.g., generic EventMulticaster, transport-level RetryPolicy) and updates TapToPay clients to use the transport seam.
• Adds PayabliSDKTestUtils as a real SPM product and migrates common fixtures from test bundles into it; updates tests/import hygiene accordingly.

Reviewed changes

Copilot reviewed 63 out of 63 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
Tests/PayabliSDKTestUtilsTests/PayabliSDKTestUtilsTests.swift	Adds smoke tests for the new PayabliSDKTestUtils product.
Tests/PayabliSDKTapToPayTests/TapToPayProviderFactoryTests.swift	Removes tests for the deleted provider factory.
Tests/PayabliSDKTapToPayTests/SecureStorageTests.swift	Updates tests to use fixtures from PayabliSDKTestUtils.
Tests/PayabliSDKTapToPayTests/PayabliTTPTests.swift	Updates tests to use fixtures from PayabliSDKTestUtils.
Tests/PayabliSDKTapToPayTests/PayabliTTPSessionInitTests.swift	Adds coverage for shared-session facade initialization.
Tests/PayabliSDKTapToPayTests/PayabliTTPEventCodeMappingTests.swift	Drops @testable usage for public mapping test.
Tests/PayabliSDKTapToPayTests/MockTapToPayProvider.swift	Deletes in-test mock moved to PayabliSDKTestUtils.
Tests/PayabliSDKTapToPayTests/MockAppAttestor.swift	Deletes in-test mock moved to PayabliSDKTestUtils.
Tests/PayabliSDKTapToPayTests/EventMulticasterTests.swift	Deletes TapToPay-local multicaster tests (now in Core).
Tests/PayabliSDKTapToPayTests/AppAttestServiceTests.swift	Updates tests to use fixtures from PayabliSDKTestUtils.
Tests/PayabliSDKCoreTests/TelemetryClientTests.swift	Updates version assertion and uses in-memory telemetry transport fixture.
Tests/PayabliSDKCoreTests/StubURLProtocol.swift	Deletes in-test StubURLProtocol moved to PayabliSDKTestUtils.
Tests/PayabliSDKCoreTests/RetryPolicyTests.swift	Updates tests for Core retry primitives and stronger error assertions.
Tests/PayabliSDKCoreTests/PayabliTransportTests.swift	Adds compile-time conformance test for the transport seam.
Tests/PayabliSDKCoreTests/PayabliSessionTests.swift	Adds tests for PayabliSession construction and URLSession injection.
Tests/PayabliSDKCoreTests/PayabliServiceTests.swift	Updates tests to use fixtures from PayabliSDKTestUtils.
Tests/PayabliSDKCoreTests/PayabliSDKCoreTests.swift	Drops @testable import where not needed.
Tests/PayabliSDKCoreTests/PayabliErrorCodeMappingTests.swift	Adds validation error code mapping test coverage.
Tests/PayabliSDKCoreTests/PayabliEnvironmentTests.swift	Gates .local assertions behind #if DEBUG.
Tests/PayabliSDKCoreTests/PayabliAuthTests.swift	Adds coverage for tokenChanges() stream emission.
Tests/PayabliSDKCoreTests/EventMulticasterTests.swift	Adds tests for the new generic Core multicaster.
Tests/PayabliSDKCoreTests/AuthenticatedTransportTests.swift	Adds tests for auth injection + 401 refresh-and-retry decorator behavior.
Sources/PayabliSDKTestUtils/StubURLProtocol.swift	Promotes StubURLProtocol to public test utility with body-stream draining.
Sources/PayabliSDKTestUtils/PayabliSDKTestUtils.swift	Adds a TestUtils namespace + bundle-derived version accessor.
Sources/PayabliSDKTestUtils/MockTapToPayProvider.swift	Adds a public mock TapToPay provider fixture.
Sources/PayabliSDKTestUtils/MockDeviceAttestationService.swift	Adds a public mock device attestation fixture.
Sources/PayabliSDKTestUtils/MockAppAttestor.swift	Adds a public mock App Attest fixture.
Sources/PayabliSDKTestUtils/InMemoryTelemetryTransport.swift	Adds an in-memory TelemetryTransport fixture.
Sources/PayabliSDKTestUtils/InMemorySecureStorage.swift	Adds an in-memory SecureStorage fixture.
Sources/PayabliSDKTelemetry/PayabliSDKTelemetry.swift	Switches telemetry module version to bundle-derived value.
Sources/PayabliSDKTapToPay/TTPTransactionClient.swift	Refactors to depend on PayabliTransport instead of raw service/auth.
Sources/PayabliSDKTapToPay/TTPConfigClient.swift	Refactors to depend on PayabliTransport and shared HTTP error mapping.
Sources/PayabliSDKTapToPay/TapToPayProviderFactory.swift	Removes unused provider factory implementation.
Sources/PayabliSDKTapToPay/SessionManager.swift	Tightens visibility of SessionManager to internal.
Sources/PayabliSDKTapToPay/SecureStorage.swift	Removes in-module in-memory storage; points tests to TestUtils fixture.
Sources/PayabliSDKTapToPay/PayabliTTP+Charge.swift	Removes open-coded 401 retry; uses session transport for updates.
Sources/PayabliSDKTapToPay/PayabliTTP.swift	Introduces session: designated init + uses shared session/transport; extracts ObjC helpers.
Sources/PayabliSDKTapToPay/PayabliSDKTapToPay.swift	Switches TapToPay module version to bundle-derived value.
Sources/PayabliSDKTapToPay/KeychainStorage.swift	Trims extra KeychainStorage methods from public surface.
Sources/PayabliSDKTapToPay/EventMulticasterAlias.swift	Adds TapToPay alias to Core EventMulticaster<PayabliTTPEvent>.
Sources/PayabliSDKTapToPay/EventMulticaster.swift	Removes TapToPay-local multicaster implementation (now in Core).
Sources/PayabliSDKTapToPay/DeviceAttestationService.swift	Adds explicit initializer to AssertionHeaders.
Sources/PayabliSDKTapToPay/AppAttestService+Defaults.swift	Demotes default hardware-id providers to internal visibility.
Sources/PayabliSDKTapToPay/AppAttestService.swift	Splits init: public convenience + internal designated for provider overrides.
Sources/PayabliSDKTapToPay/Adapters/FiservCardReader.swift	Tightens visibility of Credentials and setCredentials.
Sources/PayabliSDKTapToPay/_ObjCBridging.swift	Extracts ObjC bridging helpers from the facade file.
Sources/PayabliSDKCore/Telemetry/TelemetryClient.swift	Removes Core in-memory transport; uses Core version for sdkVersion default.
Sources/PayabliSDKCore/Public/PayabliSession.swift	Adds shared session backbone exposing auth/service and computed transport.
Sources/PayabliSDKCore/Public/PayabliEnvironment.swift	Gates .local environment behind #if DEBUG.
Sources/PayabliSDKCore/Public/PayabliConfig.swift	Converts PayabliConfig to a value type (struct).
Sources/PayabliSDKCore/PayabliSDKCore.swift	Switches Core version to bundle-derived value.
Sources/PayabliSDKCore/Networking/RetryPolicy.swift	Generalizes retry docs and fallback error type in Core.
Sources/PayabliSDKCore/Networking/PayabliTransport.swift	Introduces the transport protocol seam.
Sources/PayabliSDKCore/Networking/PayabliService.swift	Conforms to PayabliTransport and factors shared HTTP error mapping.
Sources/PayabliSDKCore/Networking/AuthenticatedTransport.swift	Adds auth-injecting decorator with 401 refresh-and-retry behavior.
Sources/PayabliSDKCore/Models/PayabliError.swift	Adds .validation error code and maps validation errors correctly.
Sources/PayabliSDKCore/Concurrency/EventMulticaster.swift	Adds generic Core EventMulticaster<Event>.
Sources/PayabliSDKCore/Auth/SessionTierValidator.swift	Demotes SessionTierValidator visibility to internal.
Sources/PayabliSDKCore/Auth/PayabliAuth.swift	Adds tokenChanges() AsyncStream and emits on refresh.
Package.swift	Adds PayabliSDKTestUtils product/target and wires tests to it.
Documentation/Plans/2026-05-06-architecture-refactor.md	Adds detailed implementation plan document.
Documentation/ArchitectureAssessment-2026-05-06.md	Adds architecture assessment + resolution status.
CLAUDE.md	Codifies an “umbrella inclusion rule” for module packaging.

Comments suppressed due to low confidence (3)

Sources/PayabliSDKTapToPay/KeychainStorage.swift:42

• Changing data(forKey:) from public to internal on a public type is a source-breaking change for any consumer calling it directly. If the goal is to tighten the public surface without breaking downstream code, consider keeping it public but deprecating it (and/or update the PR’s “No public-API breaks” statement / versioning).

    internal func data(forKey key: String) -> Data? {
        let query: [String: Any] = [
            kSecClass as String: kSecClassGenericPassword,
            kSecAttrService as String: service,
            kSecAttrAccount as String: key,

Sources/PayabliSDKTapToPay/KeychainStorage.swift:65

• The set(_ data:forKey:) overload being demoted from public to internal is a source-breaking API change for any consumer that stored raw Data in KeychainStorage. If this is intended, it should be called out in release notes / semver; otherwise consider deprecating rather than removing public access.

    internal func set(_ data: Data, forKey key: String) throws {
        let baseQuery: [String: Any] = [
            kSecClass as String: kSecClassGenericPassword,
            kSecAttrService as String: service,
            kSecAttrAccount as String: key

Sources/PayabliSDKTapToPay/KeychainStorage.swift:100

• removeAll() was previously public and is now internal, which is a source-breaking change for any external cleanup code using it. If you need to hide it, consider a deprecation period or updating the PR/release messaging to reflect this public API removal.

    internal func removeAll() {
        let query: [String: Any] = [
            kSecClass as String: kSecClassGenericPassword,
            kSecAttrService as String: service
        ]

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[thejohnnybot]

Follow-up batch landed (7 commits on top of 4651fbe):

Item	Commit	What
1	32b0147	refactor(taptopay): route AppAttestService through PayabliTransport — closes the last transport-seam bypass; device-attestation requests now flow through AuthenticatedTransport (bearer injection + 401 refresh-and-retry + typed error mapping) like every other endpoint client.
2	d971ca5	refactor(core): make AuthenticatedTransport internal — concrete decorator type hidden; reachable only via PayabliSession.transport returning any PayabliTransport.
3	(no commit)	Verified Example/PayabliDemo source uses only preserved public API (PayabliTTP(accessToken:...), PayabliTTPSessionState, PayabliTTPPaymentDetails, PayabliTTPEventToken). No demo changes required.
4	5ff4330	docs(core): add per-property docs to PayabliSession — config, auth, service get their own /// doc comments.
5	bf9e12b	test(core): verify custom URLSession is actually wired through PayabliSession — replaces the previously-vacuous XCTAssertNotNil with a real StubURLProtocol round-trip.
6	6d71c65	style(core): reorder case validation in PayabliErrorCode — alphabetical placement after userCancelled.
7	5e342f6	chore(core): precondition RetryPolicy.maxAttempts >= 1 — guards against fatal range errors on 1...0.
—	7bd33c4	docs: mark AuthenticatedTransport visibility follow-up resolved — resolution table reflects the close.

Both originally-listed follow-ups (AppAttestService+Requests bypass, AuthenticatedTransport public) are now closed. The remaining follow-up — PayabliSession.auth/.service → package — stays a separate PR since it requires package adoption verification across the test bundles.

xcodebuild test green after every commit on the iOS Simulator scheme. CI rerunning on the new HEAD.

[thejohnnybot]

Addressed @copilot-pull-request-reviewer's findings (5 commits on top of c38cc06):

Comment	Status	Commit
Package.swift — TestUtils depends on PayabliSDKTelemetry but never uses it	✅ Fixed	c5a05ae (drop the dependency)
PayabliSDKTestUtilsTests.swift — XCTAssertTrue(true) is vacuous	✅ Fixed	0d3f2f7 (assert providerId == "mock")
MockAppAttestor.swift — @unchecked Sendable + unsynchronized state	✅ Fixed	80467c4 (NSLock around all mutable state + counters)
MockTapToPayProvider.swift — same	✅ Fixed	3315518
MockDeviceAttestationService.swift — same	✅ Fixed	cd0999e
InMemoryTelemetryTransport.swift — drainBatches() should be async	❌ Declined	InMemoryTelemetryTransport is a public actor; actor methods auto-async at the call site. Marking the declaration async would suggest the implementation suspends when it doesn't. The call site already requires await.
KeychainStorage.swift — three demotions are source-breaking	⚠ Acknowledged	The three methods (data(forKey:), set(_ data:forKey:), removeAll()) were trimmed deliberately per audit Finding 19e — they were never documented, never in the SecureStorage protocol, and not advertised as the host-app surface. PR description updated to call this out explicitly under "API surface changes."

Tests green after each commit.