Repository navigation
feat(ait): add authoritative IAP SDK 3 support - #130
Conversation
|
Warning Review limit reached
Next review available in: 11 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds Apps in Toss IAP support with server-authoritative grant verification, partner API methods, runtime headers, and pending-order restoration. It also adds a local SDK mock and updates AIT applications, templates, documentation, validation, and SDK versions. ChangesApps in Toss commerce integration
Apps in Toss local development workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Game
participant AitHost
participant NativeIAP
participant GrantVerifier
Game->>AitHost: request purchase
AitHost->>NativeIAP: create purchase order
NativeIAP-->>AitHost: return purchase evidence
AitHost->>GrantVerifier: verify product grant
GrantVerifier-->>AitHost: return verification result
AitHost->>NativeIAP: complete verified grant
AitHost-->>Game: return purchase result
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
packages/game-services/src/runtime.test.ts (1)
282-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd oRPC header-forwarding coverage.
The omitted
transportvalue selects HTTP. This test does not exercise the changed oRPC branch. Add a second case withtransport: 'orpc'and assert the same headers after a purchase request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/game-services/src/runtime.test.ts` around lines 282 - 291, Add a second header-forwarding test alongside the existing HTTP case using createGameServicesRuntime with transport: 'orpc'; perform the same purchase request and assert that x-ttokdoku-player-key and x-ttokdoku-target are forwarded unchanged, while preserving the current HTTP coverage.tools/validate-starter-workflow.ts (1)
1020-1022: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate the
./local-mockpackage export.The template requires
@mpgd/adapter-ait/local-mock. This validator checks only the alias text. It does not checkadapters/ait/package.jsonfor the required./local-mockexport and itsdist/local-mocktype and runtime paths. Add that assertion with the existing adapter subpath export checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/validate-starter-workflow.ts` around lines 1020 - 1022, Extend the validator around the existing adapter subpath export checks to inspect adapters/ait/package.json and assert the ./local-mock export exists with the required dist/local-mock type and runtime paths. Keep the existing alias-text validation unchanged and use the same assertion pattern as the other adapter exports.adapters/ait/src/host.test.ts (3)
1605-1630: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
satisfiesover theascast in the IAP double.Line 1629 casts the returned object with
as AitHostDependencies['iap']. The cast suppresses compile errors ifAitIapDependenciesgains a method or changes a signature, so the double can silently drift from the host contract that lines 1561-1569 derive their aliases from. Usesatisfies AitHostDependencies['iap']to keep the structural check.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 1605 - 1630, Update createSupportedIap to use satisfies AitHostDependencies['iap'] instead of an as cast, preserving the existing object implementation while enforcing compile-time compatibility with the IAP host contract.
265-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the call counts to pin the unmapped-order skip.
toHaveBeenCalledWithpasses when any call matches. The test therefore still passes if the host also verifies or completes'unmapped-order'. Add call-count assertions so the skip of the unconfigured SKU is enforced.💚 Proposed change
expect(verifyIapProductGrant).toHaveBeenCalledWith(expect.objectContaining({ orderId: 'pending-order-1', source: 'pending-order-restore', idempotencyKey: 'apps-in-toss:purchase:pending-order-1', })); + expect(verifyIapProductGrant).toHaveBeenCalledTimes(1); expect(completeProductGrant).toHaveBeenCalledWith({ params: { orderId: 'pending-order-1' }, }); + expect(completeProductGrant).toHaveBeenCalledTimes(1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 265 - 272, Add exact call-count assertions for verifyIapProductGrant and completeProductGrant in the pending-order restore test, alongside the existing argument assertions. Ensure each mock is called only once so the test fails if the unmapped order is also verified or completed.
98-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where
onEventarrives without a preceding grant attempt.The purchase tests always call
callbacks.options.processProductGrantbeforecallbacks.onEvent. The host handles the opposite order at Line 1682 ofadapters/ait/src/host.ts, where a missinggrantAttemptsentry resolves tofalseand produces a failed purchase. That fail-closed branch carries the entitlement decision and is currently untested. Add a test that callsonEventdirectly and asserts{ status: 'failed', entitlementIds: [] }.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 98 - 187, Add a test near the existing purchase flow that invokes callbacks.onEvent directly without calling callbacks.options.processProductGrant first, using a representative successful purchase event. Await the purchase request and assert it resolves with status 'failed' and an empty entitlementIds array, covering the missing grantAttempts fail-closed behavior handled by the host.adapters/ait/src/host.ts (3)
1916-1923: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the rejected product in the configuration warnings.
Both warnings drop a configured product without reporting which entry failed. A misconfigured
productIdorskusilently disables a paid product, and the log gives no way to find it. Include the offending identifiers in the message.♻️ Proposed change
if (productId === undefined || sku === undefined || currencyCode === undefined) { - console.warn('AIT IAP product configuration is invalid; disabling one product.'); + console.warn( + 'AIT IAP product configuration is invalid; disabling one product.', + { productId: rawProduct.productId, sku: rawProduct.sku }, + ); continue; } if (byProductId.has(productId) || bySku.has(sku)) { console.warn( 'AIT IAP product configuration has a duplicate product id or SKU; disabling one product.', + { productId, sku }, ); continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1916 - 1923, Update the configuration warnings in the product validation loop to include the rejected product’s productId and sku values. Preserve the existing invalid-configuration and duplicate-configuration warning behavior while making each message identify the specific disabled entry.
1725-1756: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBound the pending-order restore loop.
The loop processes every entry in
pendingOrders.orderssequentially. Each iteration can wait up toinput.timeoutMs(25 seconds by default) forverifyAitIapProductGrant, plus thecompleteProductGrantcall. The provider controls the list length, so a long list blocks thecommerce.restorebridge request for a multiple of the timeout.Apply a maximum processed-order count, or verify a bounded number of orders concurrently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1725 - 1756, Bound processing in the pending-order restore loop around verifyAitIapProductGrant and completeProductGrant so provider-controlled list length cannot block the restore request indefinitely. Enforce a maximum number of processed orders or use bounded concurrency, while preserving existing filtering, verification, completion, and restoredEntitlements behavior.
1862-1866: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCanonicalize AIT restored-grant timestamps before storing them.
normalizeAitIapGrantTimepasses bounded provider values through unvalidated, while the fallback and other adapters already emittoISOString(), which includes theZtimezone. Parse the AIT value or replace it withnew Date().toISOString()when parsing fails to keepgrantedAtconsistently canonical in the ledger.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1862 - 1866, Update normalizeAitIapGrantTime to parse valid bounded provider values and return their canonical ISO timestamp with the Z timezone; return new Date().toISOString() when the value is empty, contains control/format characters, exceeds the length limit, or fails date parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/ait/src/host.ts`:
- Around line 478-497: Update the in-flight purchase coordination in the
purchase handling flow around purchaseAitIapProduct to key iapPurchasesInFlight
by both productId and idempotencyKey. Compute and consistently reuse the
composite key for get, set, and delete operations so requests sharing an
idempotency key across different products cannot collide.
- Around line 1636-1697: Add a terminal timeout in purchaseAitIapProduct using
the existing adMaximumDisplayMs duration, and have it call
finish(failedPurchase()) when neither onEvent nor onError settles the promise.
Register the timer only after the finish function is initialized, and clear or
otherwise clean up the timer when finish settles to prevent delayed callbacks
after completion.
- Around line 1878-1882: Update the product type mapping around
nativeProduct.type to explicitly support CONSUMABLE, NON_CONSUMABLE, and
SUBSCRIPTION, returning their corresponding visible values. Return undefined for
any other product type instead of defaulting unsupported values to subscription.
In `@apps/target-ait/README.md`:
- Around line 23-28: Update the development verification guidance near the
“game-owned devtools setup” reference to remove that obsolete workflow. Direct
users exclusively to the Apps in Toss QR sandbox or a real device, while
preserving the surrounding verification instructions.
---
Nitpick comments:
In `@adapters/ait/src/host.test.ts`:
- Around line 1605-1630: Update createSupportedIap to use satisfies
AitHostDependencies['iap'] instead of an as cast, preserving the existing object
implementation while enforcing compile-time compatibility with the IAP host
contract.
- Around line 265-272: Add exact call-count assertions for verifyIapProductGrant
and completeProductGrant in the pending-order restore test, alongside the
existing argument assertions. Ensure each mock is called only once so the test
fails if the unmapped order is also verified or completed.
- Around line 98-187: Add a test near the existing purchase flow that invokes
callbacks.onEvent directly without calling callbacks.options.processProductGrant
first, using a representative successful purchase event. Await the purchase
request and assert it resolves with status 'failed' and an empty entitlementIds
array, covering the missing grantAttempts fail-closed behavior handled by the
host.
In `@adapters/ait/src/host.ts`:
- Around line 1916-1923: Update the configuration warnings in the product
validation loop to include the rejected product’s productId and sku values.
Preserve the existing invalid-configuration and duplicate-configuration warning
behavior while making each message identify the specific disabled entry.
- Around line 1725-1756: Bound processing in the pending-order restore loop
around verifyAitIapProductGrant and completeProductGrant so provider-controlled
list length cannot block the restore request indefinitely. Enforce a maximum
number of processed orders or use bounded concurrency, while preserving existing
filtering, verification, completion, and restoredEntitlements behavior.
- Around line 1862-1866: Update normalizeAitIapGrantTime to parse valid bounded
provider values and return their canonical ISO timestamp with the Z timezone;
return new Date().toISOString() when the value is empty, contains control/format
characters, exceeds the length limit, or fails date parsing.
In `@packages/game-services/src/runtime.test.ts`:
- Around line 282-291: Add a second header-forwarding test alongside the
existing HTTP case using createGameServicesRuntime with transport: 'orpc';
perform the same purchase request and assert that x-ttokdoku-player-key and
x-ttokdoku-target are forwarded unchanged, while preserving the current HTTP
coverage.
In `@tools/validate-starter-workflow.ts`:
- Around line 1020-1022: Extend the validator around the existing adapter
subpath export checks to inspect adapters/ait/package.json and assert the
./local-mock export exists with the required dist/local-mock type and runtime
paths. Keep the existing alias-text validation unchanged and use the same
assertion pattern as the other adapter exports.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 89d6cc76-4091-4a5e-ac09-2c597a0d71d3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.sampo/changesets/venerable-bard-otso.mdadapters/ait/package.jsonadapters/ait/src/host.test.tsadapters/ait/src/host.tsadapters/ait/src/local-mock.test.tsadapters/ait/src/local-mock.tsapps/target-ait/README.mdapps/target-ait/package.jsonapps/target-ait/vite.config.tsdocs/ROADMAP_CHECKLIST.mdpackages/cli/templates/phaser-game/README.mdpackages/cli/templates/phaser-game/apps/target-ait/package.jsonpackages/cli/templates/phaser-game/apps/target-ait/vite.config.tspackages/cli/templates/phaser-game/package.jsonpackages/cli/templates/phaser-game/pnpm-workspace.yamlpackages/game-services/src/apps-in-toss-partner-api.test.tspackages/game-services/src/apps-in-toss-partner-api.tspackages/game-services/src/runtime.test.tspackages/game-services/src/runtime.tspnpm-workspace.yamltools/validate-starter-workflow.ts
💤 Files with no reviewable changes (2)
- packages/cli/templates/phaser-game/pnpm-workspace.yaml
- pnpm-workspace.yaml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0a6e0b35b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed all actionable review feedback in db759eb:
Validation: pnpm check, adapter/game-services tests, Apps in Toss production build, and local OCR review all pass. @coderabbitai review |
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db759eb3b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up in eea5902: the generic IAP bridge now deliberately hides Apps in Toss Validated with the adapter suite (73 tests), full @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eea59027cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up in 6c4b79c addresses the remaining review findings:
Validation: adapter suite (75 tests), full @codex review |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
adapters/ait/src/host.test.ts (1)
508-552: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the restore truncation branch.
This test proves that ineligible orders do not consume the limit. No test drives the opposite case: more than
maximumPendingIapOrders(20) eligible orders. The truncation branch inadapters/ait/src/host.tsat Lines 1800-1805, including the per-launch limit warning and theignoredOrderCountvalue, stays uncovered. Add a case with 21 mapped orders and assert thatverifyIapProductGrantruns exactly 20 times.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 508 - 552, Add a restore test alongside the existing pending-order coverage with 21 eligible mapped orders, exceeding maximumPendingIapOrders. Invoke commerce.restore and assert verifyIapProductGrant is called exactly 20 times, exercising the truncation branch and ignoredOrderCount warning path.adapters/ait/src/host.ts (4)
1653-1667: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the entitlement read failure.
readAitIapEntitlementsreturns an empty array for both "no entitlements" and "read failed". The caller cannot separate the two cases. Other failure paths in this file emit a diagnostic (see Line 1801 and Line 2020). Add a warning so a failing authority read is visible in the field.♻️ Proposed diagnostic
try { return [...await reader()]; - } catch { + } catch (error) { // The game authority owns durable purchase state. A read failure must not // turn stale local data into a visible entitlement. + console.warn('AIT IAP entitlement read failed; reporting no entitlements.', error); return []; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1653 - 1667, Update readAitIapEntitlements to capture the caught error and emit a warning diagnostic before returning an empty array, matching the existing diagnostic conventions used by nearby failure paths such as those around lines 1801 and 2020.
2003-2012: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCheck the subscription type before the display-text validation.
Lines 1997-2002 return
undefinedwhen any display field is invalid. ASUBSCRIPTIONproduct with a malformeddisplayNametherefore skips the subscription warning, and the diagnostic becomes dependent on unrelated field quality. Move theSUBSCRIPTIONbranch above the display-text checks to make the log deterministic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 2003 - 2012, Move the nativeProduct.type === 'SUBSCRIPTION' check and its warning/return branch before the display-text validation in the surrounding product handling flow. Ensure subscription products always emit the subscription-unavailable warning and return undefined regardless of malformed display fields, while preserving existing validation for non-subscription products.
1688-1698: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making the purchase session deadline configurable.
The session guard uses the module constant
defaultIapPurchaseSessionTimeoutMs. Every other terminal bound in this file is an option (adMaximumDisplayMs,iapProductGrantTimeoutMs). A host that must shorten a stuck purchase UI cannot do so today. Pass the value throughAitIapPurchaseInputif you want parity with the ad path. The current fixed default is safe, so this is optional.Also applies to: 1727-1730
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1688 - 1698, Make the purchase-session timeout configurable instead of always using defaultIapPurchaseSessionTimeoutMs. Add an optional timeout field to AitIapPurchaseInput, propagate it through the purchase flow, and have the session deadline use that value with the existing default as fallback, preserving current behavior when unspecified.
1799-1834: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the total restore duration, not only the order count.
The loop verifies up to
maximumPendingIapOrders(20) orders one after another. EachverifyAitIapProductGrantcall can consume the fulltimeoutMs, whichnormalizeIapProductGrantTimeoutpermits up to 25 s. In the worst casecommerce.restoreblocks the bridge request for about 500 s, and the game UI has no terminal signal. Add an overall deadline for the restore pass, or verify with bounded concurrency and stop when the deadline passes.♻️ Sketch of a total restore deadline
+ const restoreDeadline = Date.now() + input.timeoutMs * 4; for (const { order, product } of orders) { + if (Date.now() >= restoreDeadline) { + console.warn('AIT IAP pending-order restore reached its time budget.', { + remainingOrderCount: orders.length - restoredEntitlements.length, + }); + break; + } const granted = await verifyAitIapProductGrant({🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.ts` around lines 1799 - 1834, Bound the pending-order restore pass by an overall deadline in the loop handling orders from eligibleOrders, rather than relying only on maximumPendingIapOrders and per-call timeoutMs. Establish the deadline before processing orders, stop starting further verification or completion work once it expires, and preserve successful entitlement restoration for orders completed before the deadline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/ait/src/host.ts`:
- Around line 1863-1879: Update isAitOneTimeIapProduct to wrap
dependencies.iap.getProductItemList() in the same bounded timeout race used by
prepareAitIap and verifyAitIapProductGrant, using the configured grant timeout.
Preserve the existing SKU/type checks and false-on-error behavior. Also inspect
listAitIapProducts and getPendingOrders for equivalent unbounded native catalog
calls and apply the same bound where required.
---
Nitpick comments:
In `@adapters/ait/src/host.test.ts`:
- Around line 508-552: Add a restore test alongside the existing pending-order
coverage with 21 eligible mapped orders, exceeding maximumPendingIapOrders.
Invoke commerce.restore and assert verifyIapProductGrant is called exactly 20
times, exercising the truncation branch and ignoredOrderCount warning path.
In `@adapters/ait/src/host.ts`:
- Around line 1653-1667: Update readAitIapEntitlements to capture the caught
error and emit a warning diagnostic before returning an empty array, matching
the existing diagnostic conventions used by nearby failure paths such as those
around lines 1801 and 2020.
- Around line 2003-2012: Move the nativeProduct.type === 'SUBSCRIPTION' check
and its warning/return branch before the display-text validation in the
surrounding product handling flow. Ensure subscription products always emit the
subscription-unavailable warning and return undefined regardless of malformed
display fields, while preserving existing validation for non-subscription
products.
- Around line 1688-1698: Make the purchase-session timeout configurable instead
of always using defaultIapPurchaseSessionTimeoutMs. Add an optional timeout
field to AitIapPurchaseInput, propagate it through the purchase flow, and have
the session deadline use that value with the existing default as fallback,
preserving current behavior when unspecified.
- Around line 1799-1834: Bound the pending-order restore pass by an overall
deadline in the loop handling orders from eligibleOrders, rather than relying
only on maximumPendingIapOrders and per-call timeoutMs. Establish the deadline
before processing orders, stop starting further verification or completion work
once it expires, and preserve successful entitlement restoration for orders
completed before the deadline.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e330097f-7693-41ab-bd4a-a52d381fd875
📒 Files selected for processing (7)
.sampo/changesets/venerable-bard-otso.mdadapters/ait/src/host.test.tsadapters/ait/src/host.tsapps/target-ait/README.mdapps/target-ait/vite.config.tspackages/game-services/src/runtime.test.tstools/validate-starter-workflow.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/target-ait/README.md
- apps/target-ait/vite.config.ts
- .sampo/changesets/venerable-bard-otso.md
- tools/validate-starter-workflow.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c4b79c893
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up in 818348b: native IAP catalog reads, pending-order reads, and grant-completion acknowledgements now share the configured terminal deadline. Every timeout returns a fail-closed result, and restore still leaves provider state pending when completion is ambiguous. Added coverage for hung catalog listing/purchase, hung pending-order retrieval, and hung provider completion. Validation: 77 adapter tests, full @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 818348b855
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Follow-up review fixes are now pushed through bc6a3c7.
Previously reported entitlement-reader, CLI changeset, subscription, callback-loss, and idempotency findings are addressed by these commits or earlier follow-ups. Validation: pnpm check, adapter suite (86 tests), game-services suite, and Apps in Toss production build all pass. Local OCRs LLM connected but repeatedly aborted during the planner stage for these commits; OCR delegate mode reviewed the selected source file and rules, while the adapter suite covers the excluded test file. @codex review |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
adapters/ait/src/host.test.ts (4)
2392-2415: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the overlapping helper inputs mutually exclusive.
productsandgetProductItemListboth configure the catalog, andpendingOrdersandgetPendingOrdersboth configure pending orders. If a test passes both, the function override silently wins and the data field is ignored. That produces a test that asserts the wrong setup with no signal. Model each pair as one discriminated input, or throw when both are supplied.♻️ Suggested guard
} = {}): AitHostDependencies['iap'] { + if (input.products !== undefined && input.getProductItemList !== undefined) { + throw new Error('Pass either products or getProductItemList, not both.'); + } + if (input.pendingOrders !== undefined && input.getPendingOrders !== undefined) { + throw new Error('Pass either pendingOrders or getPendingOrders, not both.'); + } const iap = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 2392 - 2415, Update createSupportedIap so each overlapping input pair is mutually exclusive: products versus getProductItemList, and pendingOrders versus getPendingOrders. Model the options as discriminated unions or validate and throw when both members of either pair are provided, preventing one configuration from silently overriding the other.
256-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the storage key literals into shared test constants.
The keys
mpgd:ait:iap-completed-purchase-index:v1andmpgd:ait:iap-purchase-attempt:v1:...are repeated in this test and in the retention and stale-marker tests (Lines 303, 308, 310, 632, 675). A single constant plus a small key builder keeps the tests aligned when the host renames a key version.♻️ Suggested helpers
const COMPLETED_INDEX_KEY = 'mpgd:ait:iap-completed-purchase-index:v1'; const attemptKey = (productId: string, idempotencyKey: string): string => `mpgd:ait:iap-purchase-attempt:v1:${productId}:${idempotencyKey}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 256 - 261, Define shared test constants for the completed purchase index key and an attempt-key builder, then replace the repeated literals in the purchase, retention, and stale-marker tests with those helpers. Preserve the existing product and idempotency-key values while ensuring all assertions and lookups use the centralized definitions.
946-952: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse a shared IAP options factory across the suite.
commonOptionsremoves the repeated option block in this test. The same four fields (iapProducts,prepareIap,verifyIapProductGrant,readIapEntitlements) are repeated inline in the tests at Lines 216-222, 267-271, 587-591, 641-645, 682-686, 838-842, 911-915, and 995-999. Promote a module-level factory and let each test override only the fields it exercises.♻️ Suggested helper
function createIapOptions( overrides: Partial<InstallAitHostBridgeOptions> = {}, ): InstallAitHostBridgeOptions { return { iapProducts: [{ productId: 'HINT_PACK_5', sku: 'ait.ttokdoku.hints.5' }], prepareIap: async () => true, verifyIapProductGrant: async () => true, readIapEntitlements: async () => [], ...overrides, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 946 - 952, Introduce a module-level createIapOptions factory returning the shared IAP defaults, typed as InstallAitHostBridgeOptions with Partial overrides. Replace the repeated inline option blocks throughout the host test suite, including commonOptions, with this factory and pass only test-specific fields through overrides; preserve each test’s existing behavior and timeout settings.
587-616: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet
iapProductGrantTimeoutMsexplicitly instead of waiting for the 30-minute default to expire.
30 * 60_000depends ondefaultIapProductGrantTimeoutMsinstead of a local test option. Use the same small explicit timeout as the other IAP timeout tests, e.g.iapProductGrantTimeoutMs: 10, and advance only10ms.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/ait/src/host.test.ts` around lines 587 - 616, Update the createAitHostBridge configuration in the lost-terminal-callback test to set iapProductGrantTimeoutMs explicitly to 10, matching the other IAP timeout tests, and advance timers by 10 ms instead of 30 minutes. Keep the purchase and callback assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@adapters/ait/src/host.test.ts`:
- Around line 2392-2415: Update createSupportedIap so each overlapping input
pair is mutually exclusive: products versus getProductItemList, and
pendingOrders versus getPendingOrders. Model the options as discriminated unions
or validate and throw when both members of either pair are provided, preventing
one configuration from silently overriding the other.
- Around line 256-261: Define shared test constants for the completed purchase
index key and an attempt-key builder, then replace the repeated literals in the
purchase, retention, and stale-marker tests with those helpers. Preserve the
existing product and idempotency-key values while ensuring all assertions and
lookups use the centralized definitions.
- Around line 946-952: Introduce a module-level createIapOptions factory
returning the shared IAP defaults, typed as InstallAitHostBridgeOptions with
Partial overrides. Replace the repeated inline option blocks throughout the host
test suite, including commonOptions, with this factory and pass only
test-specific fields through overrides; preserve each test’s existing behavior
and timeout settings.
- Around line 587-616: Update the createAitHostBridge configuration in the
lost-terminal-callback test to set iapProductGrantTimeoutMs explicitly to 10,
matching the other IAP timeout tests, and advance timers by 10 ms instead of 30
minutes. Keep the purchase and callback assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f094403c-96be-4bd5-b79a-0708c1add5b6
📒 Files selected for processing (2)
adapters/ait/src/host.test.tsadapters/ait/src/host.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- adapters/ait/src/host.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc6a3c78d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bc6a3c7 to
9297a3b
Compare
|
@codex review 최신 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9297a3bc13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Re-review requested for |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f9ffab588
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Re-review requested for |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37d06308bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Re-review requested for |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review Final re-review request for current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ccb551ba8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (markerDeletionStarted) { | ||
| // Cancellation won the serialization race. Do not let a later queued | ||
| // grant callback commit after its retry barrier starts disappearing. | ||
| return Promise.resolve(false); |
There was a problem hiding this comment.
When the SDK reports cancellation and then invokes a queued processProductGrant for an order it already dispatched, this branch returns false without recording the orderId, while the previously started marker removal continues and can resolve the purchase as cancelled. Retrying the same client idempotency key then sees no marker and opens a second native order, even though the first paid order can remain in getPendingOrders() and later be verified and granted during restore. Fresh evidence in the current head is that the new deletion lock discards the late provider-order correlation rather than restoring the retry barrier; preserve that correlation as pending while still deferring the grant to backend reconciliation.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
| const { order, product } = selectedOrder; | ||
| const cursorWriteTimeoutMs = getRemainingReconciliationTimeout(); | ||
| if (cursorWriteTimeoutMs === 0) { | ||
| break; | ||
| } | ||
| const cursorAdvanced = await writeAitIapPendingOrderCursor( | ||
| input.dependencies.storage, | ||
| order.orderId, |
There was a problem hiding this comment.
When the native Storage.setItem for the cursor rejects or times out, cursorAdvanced is false and this branch exits before verifying even the first paid pending order. If that storage failure persists, every restore can successfully fetch getPendingOrders() yet skip all backend verification and completion, leaving orders that have not already reached the ledger pending indefinitely. Fresh evidence after the earlier cursor fix is that the current pre-verification write is now a hard prerequisite; treat the cursor as best-effort fairness bookkeeping and still process a bounded current order when it cannot be persisted.
AGENTS.md reference: AGENTS.md:L29-L29
Useful? React with 👍 / 👎.
Summary
Validation
mise exec -- pnpm checkmise exec -- pnpm --filter @mpgd/game-services testmise exec -- pnpm --filter @mpgd/adapter-ait testmise exec -- pnpm --dir apps/target-ait buildb0a6e0b(no actionable findings)Summary by CodeRabbit
New Features
Improvements
Documentation