Repository navigation
feat: add Verse8 Agent8 commerce boundary - #85
Conversation
|
@codex review @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds Verse8 VXShop commerce integration, an Agent8 server-side consume-once grant service, Verse8 catalog enablement, pending purchase handling, runtime validation, package exports, tests, smoke-test updates, and commerce documentation. ChangesVerse8 commerce flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant VXShop
participant Agent8
participant UserState
Client->>VXShop: Open shop and buy product
VXShop-->>Client: Pending purchase status
VXShop->>Agent8: $onItemPurchased event
Agent8->>UserState: Lock account and write grant marker
UserState-->>Agent8: Updated commerce state
Agent8-->>Client: Authenticated entitlement snapshot
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
adapters/verse8/src/agent8.test.ts (1)
175-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise real concurrent lock behavior in this suite.
This stub ignores the lock key and runs immediately, so tests cover only sequential retries. Add a keyed mutex fixture and a concurrent
Promise.alltest asserting one state update and one grant.🤖 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/verse8/src/agent8.test.ts` around lines 175 - 177, Replace the immediate callback stub in the lock fixture with a keyed mutex that serializes callbacks sharing the same key. Extend the relevant agent8 tests with a concurrent Promise.all scenario, asserting that concurrent retries produce exactly one state update and one grant.adapters/verse8/src/agent8.ts (1)
97-99: 🚀 Performance & Scalability | 🔵 TrivialPlan for an indexed, scalable consume-once ledger.
Each purchase scans, validates, appends, and rewrites the account’s complete permanent history. Repeat consumables make processing and state payloads grow linearly for the account.
Verify Agent8 state limits and transactional storage options, then use an indexed ledger while preserving atomic grant-and-marker writes.
Also applies to: 124-131
🤖 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/verse8/src/agent8.ts` around lines 97 - 99, Replace the linear current.purchases.find lookup and full-history rewrites in the Agent8 purchase-consumption flow with an indexed consume-once ledger suitable for repeat consumables. Verify Agent8 state-size limits and available transactional storage, and persist consumption markers through the indexed store while preserving atomicity between granting the purchase and recording its marker.
🤖 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/verse8/src/agent8.ts`:
- Around line 247-265: Update the entitlement-grant flow around the existing
entitlement lookup to reject grants whose quantity is greater than one before
recording or consuming the purchase ID. Ensure only single-quantity purchases
proceed to add the entitlement and return entitlementIds, while multi-quantity
purchases are rejected without creating a permanent marker.
In `@adapters/verse8/src/index.ts`:
- Around line 373-400: Move the canOpenShop evaluation in purchase inside the
existing try/catch boundary so exceptions from the caller-provided callback are
converted to failedPurchase(). Preserve the current product lookup and
fail-closed behavior for false or unavailable shop access, while keeping
initialization, refresh, item validation, and purchase handling unchanged.
In `@packages/game-services/src/client.ts`:
- Around line 216-218: Update the reason selection in the purchase analytics
tracking flow to return 'agent8_purchase_event_pending' only when
purchase.status is strictly 'pending'; preserve
'verse8_grants_require_agent8_purchase_event' for completed purchases and assign
the appropriate non-pending handling for canceled or failed statuses.
---
Nitpick comments:
In `@adapters/verse8/src/agent8.test.ts`:
- Around line 175-177: Replace the immediate callback stub in the lock fixture
with a keyed mutex that serializes callbacks sharing the same key. Extend the
relevant agent8 tests with a concurrent Promise.all scenario, asserting that
concurrent retries produce exactly one state update and one grant.
In `@adapters/verse8/src/agent8.ts`:
- Around line 97-99: Replace the linear current.purchases.find lookup and
full-history rewrites in the Agent8 purchase-consumption flow with an indexed
consume-once ledger suitable for repeat consumables. Verify Agent8 state-size
limits and available transactional storage, and persist consumption markers
through the indexed store while preserving atomicity between granting the
purchase and recording its marker.
🪄 Autofix (Beta)
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
Run ID: c91011d2-a847-4b08-8970-ce777545ff78
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.sampo/changesets/verse8-agent8-commerce.mdREADME.mdadapters/verse8/package.jsonadapters/verse8/src/agent8.test.tsadapters/verse8/src/agent8.tsadapters/verse8/src/index.test.tsadapters/verse8/src/index.tsadapters/verse8/test/agent8-dist-import.mjsdocs/GAME_SERVICES_BACKEND.mddocs/VERSE8_COMMERCE.mdpackages/catalog/catalog.jsonpackages/cli/templates/phaser-game/README.mdpackages/game-services/src/client.test.tspackages/game-services/src/client.tspackages/target-config/src/effective.tspackages/target-config/targets.jsontools/smoke/adapter-effective-config.tstools/smoke/effective-target-config.tstools/smoke/game-owned-catalog.tstools/smoke/target-config-runtime.tstsconfig.base.json
|
Addressed the review findings in e33dff6:
The permanent marker remains in the same Agent8 user-state update as the grant. Moving it to a separate collection would lose that atomic grant-and-marker property because the public Agent8 contract does not expose a cross-store transaction. Validation completed: pnpm check, package build, 28 Verse8 adapter tests, both Verse8 dist subpath imports, GameServices client test, and diff checks. @codex review |
|
Your rationale on keeping the permanent marker in the same Agent8 user-state update (to preserve atomicity, since the public Agent8 contract doesn't expose a cross-store transaction) makes sense and I'll keep that in mind for this review. ✏️ Learnings added
✅ Action performedReview finished.
|
![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: e33dff6d71
ℹ️ 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 the Codex P2 finding in 0358b6e. The Agent8 service now checks the permanent consume marker under the account lock before requiring a current catalog mapping. An identical retry still returns its stored logical product and alreadyProcessed result after the product is retired, while a genuinely new unknown product remains fail-closed. The regression test shares persisted state between the active and retired catalog services. Validation completed: pnpm check, 28 Verse8 adapter tests, catalog validation, package type check, and diff checks. @codex review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
cd /tmp && rm -rf repo && git clone --depth 50 https://github.com/imjlk/mpgd-kit.git repo 2>&1 | tail -5
cd repo && git fetch origin pull/85/head:pr85 2>&1 | tail -5
git checkout pr85 2>&1 | tail -5
git log --oneline -10Length of output: 1099 🏁 Script executed: #!/bin/bash
cd /tmp/repo
git show 0358b6e --stat
echo "---diff---"
git show 0358b6e -- adapters/verse8/src/agent8.tsLength of output: 2868 🏁 Script executed: #!/bin/bash
cd /tmp/repo
git show 0358b6e -- adapters/verse8/src/agent8.test.tsLength of output: 1323 This looks solid. The fix correctly moves the "unknown product" check after the idempotency lookup, so a retry of an already-processed purchase now returns its stored Nice, targeted fix — thanks for the update, ✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs 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". |
Summary
Security boundary
Validation
Release
Includes a Sampo changeset for @mpgd/adapter-verse8, @mpgd/catalog, @mpgd/game-services, @mpgd/target-config, and @mpgd/cli. The Sampo release dry-run is restricted to main by repository policy, so the feature-branch invocation was expectedly rejected before mutation.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation