Skip to content

Fix redeem auth race during startup#457

Open
anglinb wants to merge 1 commit intodevelopfrom
fix/redeem-auth-race
Open

Fix redeem auth race during startup#457
anglinb wants to merge 1 commit intodevelopfrom
fix/redeem-auth-race

Conversation

@anglinb
Copy link
Copy Markdown
Collaborator

@anglinb anglinb commented Mar 26, 2026

Summary

  • configure the dependency container's API key synchronously before any async startup work runs
  • keep /redeem from racing ahead with Authorization: Bearer when ReceiptManager registers the app transaction id early
  • add a regression test covering the synchronous API-key setup invariant

Why

ReceiptManager starts setAppTransactionId() during dependency construction. If that resolves quickly after Superwall.isInitialized becomes true, it can call redeem(.existingCodes) before the old async startup task had populated storage.apiKey, which means the shared header builder would emit an empty bearer token.

Testing

  • swift test --filter StorageTests (fails in this environment because the active developer directory is Command Line Tools and the package imports UIKit: no such module "UIKit")

Greptile Summary

This PR fixes a startup race condition where ReceiptManager could call redeem(.existingCodes) before storage.apiKey was populated, because the previous code set the key inside an async Task rather than synchronously during initialization.\n\nKey changes:\n- Introduces Superwall.makeDependencyContainer(...) — a static factory that constructs DependencyContainer and immediately calls storage.configure(apiKey:) synchronously before any async work begins.\n- The private convenience init now delegates container creation to this factory, removing the storage.configure call from inside the Task.\n- Adds a regression test asserting that storage is configured synchronously after makeDependencyContainer returns — though the test uses XCTestCase rather than the project-required Swift Testing framework (CLAUDE.md).

Confidence Score: 4/5

Safe to merge — the fix is minimal, well-targeted, and correctly resolves the race condition with no risky side effects.

The core change is straightforward and correct: moving storage.configure(apiKey:) from inside an async Task to synchronous initialization eliminates the race. The only non-blocking issue is that the new regression test uses XCTest instead of the project-mandated Swift Testing framework.

Tests/SuperwallKitTests/Storage/StorageTests.swift — test should be rewritten using Swift Testing framework per CLAUDE.md.

Important Files Changed

Filename Overview
Sources/SuperwallKit/Superwall.swift Moves storage.configure(apiKey:) from inside the async Task into a new synchronous makeDependencyContainer factory, correctly eliminating the race where ReceiptManager could call redeem with an empty bearer token before the API key was set.
Tests/SuperwallKitTests/Storage/StorageTests.swift Adds a regression test for the synchronous API-key setup invariant, but uses XCTest (XCTestCase / XCTAssertEqual) instead of the project-required Swift Testing framework.

Sequence Diagram

sequenceDiagram
    participant C as Caller (configure)
    participant S as Superwall.init
    participant MDC as makeDependencyContainer
    participant DC as DependencyContainer.init
    participant ST as Storage
    participant RM as ReceiptManager

    Note over S,RM: After fix — synchronous path
    C->>S: Superwall.configure(apiKey:)
    S->>MDC: makeDependencyContainer(apiKey:)
    MDC->>DC: DependencyContainer.init()
    DC->>ST: Storage.init()
    DC->>RM: ReceiptManager.init() [may start async work]
    DC-->>MDC: dependencyContainer
    MDC->>ST: storage.configure(apiKey:) synchronous before Task
    MDC-->>S: dependencyContainer (apiKey already set)
    S->>S: Task { recordAppInstall, fetchConfig }
    Note right of RM: If ReceiptManager calls redeem() here storage.apiKey is already populated
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: Tests/SuperwallKitTests/Storage/StorageTests.swift
Line: 12-17

Comment:
**New test uses XCTest instead of Swift Testing**

`CLAUDE.md` explicitly requires: *"This project uses Swift's Testing framework (not XCTest) for all unit tests. Always use the Testing framework when writing new tests."*

The new test was added to the existing `XCTestCase` subclass rather than creating a new Swift Testing file. It should live in its own file using `import Testing` with `@Test` and `#expect`.

**Context Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=ae0afcf7-0b55-482b-9764-29f361e46714))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Fix redeem auth race during startup" | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

  • Context used - CLAUDE.md (source)

Comment on lines 12 to +17
class StorageTests: XCTestCase {
func test_makeDependencyContainer_configuresApiKeySynchronously() {
let dependencyContainer = Superwall.makeDependencyContainer(apiKey: "pk_test_123")

XCTAssertEqual(dependencyContainer.storage.apiKey, "pk_test_123")
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 New test uses XCTest instead of Swift Testing

CLAUDE.md explicitly requires: "This project uses Swift's Testing framework (not XCTest) for all unit tests. Always use the Testing framework when writing new tests."

The new test was added to the existing XCTestCase subclass rather than creating a new Swift Testing file. It should live in its own file using import Testing with @Test and #expect.

Context Used: CLAUDE.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: Tests/SuperwallKitTests/Storage/StorageTests.swift
Line: 12-17

Comment:
**New test uses XCTest instead of Swift Testing**

`CLAUDE.md` explicitly requires: *"This project uses Swift's Testing framework (not XCTest) for all unit tests. Always use the Testing framework when writing new tests."*

The new test was added to the existing `XCTestCase` subclass rather than creating a new Swift Testing file. It should live in its own file using `import Testing` with `@Test` and `#expect`.

**Context Used:** CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=ae0afcf7-0b55-482b-9764-29f361e46714))

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant