Repository navigation
feat: verify Verse8 ad rewards server-side - #84
Conversation
|
@codex review @coderabbitai review |
📝 WalkthroughWalkthroughVerse8 gains rewarded and interstitial ad support with placement resolution, client evidence, consume-once server verification, Worker routing, and target-specific configuration. Reward grants require verified server evidence, while purchases and leaderboard submissions remain unsupported for Verse8. ChangesVerse8 target support
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant GameServicesClient
participant Verse8PlatformGateway
participant GameServicesWorker
participant Verse8AdsVerifier
participant RewardLedger
GameServicesClient->>Verse8PlatformGateway: showRewarded placement
Verse8PlatformGateway->>Verse8PlatformGateway: collect requestId and create evidence
GameServicesClient->>GameServicesWorker: claimAdReward with evidence
GameServicesWorker->>Verse8AdsVerifier: consume requestId
Verse8AdsVerifier-->>GameServicesWorker: verified record
GameServicesWorker->>RewardLedger: decide catalog-backed grant
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
|
✅ 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: 31619520fc
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/game-services-worker/src/handler.ts (1)
249-261: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not let Verse8 credentials disable the aggregate verifier.
When
GAME_SERVICES_EVIDENCE_VERIFIERandVERSE8_ADS_VERIFIER_AUTHORIZATIONare both set, Line 251 enters target-specific mode. Android, iOS, and AIT then resolve no binding and fail closed instead of using the aggregate verifier.Treat built-in Verse8 configuration separately: preserve the aggregate binding for non-Verse8 targets while using the built-in verifier only as the Verse8 fallback.
Also applies to: 269-274
🤖 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 `@apps/game-services-worker/src/handler.ts` around lines 249 - 261, Update the verifier selection logic around hasTargetSpecificEvidenceVerifierBinding and createWorkerEvidenceVerifier so built-in VERSE8_ADS_VERIFIER_AUTHORIZATION does not trigger target-specific mode when GAME_SERVICES_EVIDENCE_VERIFIER is also configured. Preserve the aggregate binding for Android, iOS, and AIT, while selecting the built-in verse8Verifier only as the fallback for Verse8 targets.
🤖 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/index.ts`:
- Around line 54-58: Update Verse8PlatformGatewayOptions and the gateway
capability setup so ads are not reported as available when resolveAdPlacementId
is absent. Either make resolveAdPlacementId required or conditionally disable
all ad capabilities when it is undefined, while preserving the existing
unavailable behavior of the show methods.
In `@apps/game-services-worker/wrangler.toml`:
- Around line 26-30: Update the Verse8 rewarded-ad configuration comments near
GAME_SERVICES_VERSE8_EVIDENCE_VERIFIER to state that this binding can enable
grants as an alternative to the VERSE8_ADS_VERIFIER_AUTHORIZATION credential,
and remove the implication that the secret is always required.
In `@docs/GAME_SERVICES_BACKEND.md`:
- Around line 161-167: Update the Verse8 verifier configuration documentation to
consistently use the Worker variable name VERSE8_ADS_VERIFIER_AUTHORIZATION,
including any setup instructions or references in this section. Remove or
replace the incorrect VERSE8_ADS_VERIFIER_AUTH name while preserving the
guidance that the value is the complete server Authorization header.
In `@packages/game-services/src/types.ts`:
- Around line 263-265: Remove Verse8 from the accepted targets in
assertLeaderboardTarget in packages/game-services/src/types.ts, including its
validation message. Also remove Verse8 from the LeaderboardLedgerTarget type and
runtime validation in backend/leaderboard-ledger/src/index.ts; both sites must
reject unsupported Verse8 leaderboard submissions.
- Around line 12-15: The shared GameServicesStoreTarget contract must not treat
Verse8 as a purchase store target. In packages/game-services/src/types.ts at
lines 12-15, remove 'verse8' or split the purchase and rewarded-ad target types
so purchase verification excludes it; in packages/game-services/src/client.ts at
lines 501-503, replace store-target classification for Verse8 with the
ad-specific predicate, preserving Verse8 support only for rewarded-ad flows.
---
Outside diff comments:
In `@apps/game-services-worker/src/handler.ts`:
- Around line 249-261: Update the verifier selection logic around
hasTargetSpecificEvidenceVerifierBinding and createWorkerEvidenceVerifier so
built-in VERSE8_ADS_VERIFIER_AUTHORIZATION does not trigger target-specific mode
when GAME_SERVICES_EVIDENCE_VERIFIER is also configured. Preserve the aggregate
binding for Android, iOS, and AIT, while selecting the built-in verse8Verifier
only as the fallback for Verse8 targets.
🪄 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: e86be4c1-93e2-4328-987c-099500063f07
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.sampo/changesets/verse8-ads-verification.mdREADME.mdadapters/verse8/package.jsonadapters/verse8/src/ads-contract.tsadapters/verse8/src/index.test.tsadapters/verse8/src/index.tsadapters/verse8/src/server.test.tsadapters/verse8/src/server.tsadapters/verse8/test/server-dist-import.mjsapps/game-services-worker/package.jsonapps/game-services-worker/src/handler.tsapps/game-services-worker/src/index.test.tsapps/game-services-worker/wrangler.tomlbackend/ad-reward-ledger/src/index.tsbackend/leaderboard-ledger/src/index.tsbackend/purchase-verifier/src/index.tsdocs/GAME_SERVICES_BACKEND.mdexamples/phaser-starter/src/platform/buildGateways/verse8.tspackages/catalog/placements.jsonpackages/catalog/src/index.tspackages/cli/templates/phaser-game/src/platform/buildGateways/verse8.tspackages/game-services/src/client.tspackages/game-services/src/runtime.tspackages/game-services/src/types.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.tstools/validate-target-config.tstsconfig.base.json
💤 Files with no reviewable changes (1)
- tools/validate-target-config.ts
|
Addressed the actionable review findings in f6277f0:
The endpoint and environment-name findings were also re-verified. Validation completed: @codex review |
|
ヽ(・ω・´ﻭ ✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/game-services/src/client.ts (1)
533-538: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExplicitly list supported targets for better future-proofing.
Using negation (
target !== 'verse8') correctly mirrors the current type definitions, but it can be fragile. If a new unsupported target (e.g.,'tauri') is added toGameServicesLedgerTargetin the future, this function would incorrectly returntruefor it.Consider enumerating the supported targets explicitly as done in
isGameServicesAdRewardTargetto safely guard against future target expansions.♻️ Proposed refactor
function isGameServicesLeaderboardTarget( target: GameServicesLedgerTarget, ): target is GameServicesLeaderboardTarget { - return target !== 'verse8'; + return target === 'browser' || target === 'android' || target === 'ios' || target === 'ait' || target === 'reddit'; }🤖 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/client.ts` around lines 533 - 538, Update isGameServicesLeaderboardTarget to explicitly return true only for the currently supported leaderboard target values, matching the allowlist approach used by isGameServicesAdRewardTarget. Remove the negated exclusion check so future additions to GameServicesLedgerTarget are not implicitly accepted.
🤖 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 `@packages/game-services/src/client.ts`:
- Around line 533-538: Update isGameServicesLeaderboardTarget to explicitly
return true only for the currently supported leaderboard target values, matching
the allowlist approach used by isGameServicesAdRewardTarget. Remove the negated
exclusion check so future additions to GameServicesLedgerTarget are not
implicitly accepted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b851a99c-dee8-4f25-8112-85d19e334b91
📒 Files selected for processing (14)
adapters/verse8/src/index.test.tsadapters/verse8/src/index.tsadapters/verse8/src/server.test.tsapps/game-services-worker/src/handler.tsapps/game-services-worker/src/index.test.tsapps/game-services-worker/wrangler.tomlbackend/game-services/src/index.test.tsbackend/game-services/src/index.tsdocs/GAME_SERVICES_BACKEND.mdpackages/game-services/src/client.test.tspackages/game-services/src/client.tspackages/game-services/src/types.tstools/smoke/adapter-effective-config.tstools/smoke/target-config-runtime.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- adapters/verse8/src/server.test.ts
- apps/game-services-worker/wrangler.toml
- docs/GAME_SERVICES_BACKEND.md
- tools/smoke/target-config-runtime.ts
- apps/game-services-worker/src/index.test.ts
- adapters/verse8/src/index.test.ts
|
Codex Review: Didn't find any major issues. 🎉 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
The client callback supplies correlation evidence only. Reward grants remain authoritative in the Game Services backend: the Worker consumes the Verse8 verification record, matches it to the catalog placement and player, and then lets the ledger decide the grant. Verse8 purchases and leaderboards remain unavailable in this PR.
The built-in Worker verifier is enabled only when
VERSE8_ADS_VERIFIER_AUTHORIZATIONis configured as a secret. A target-specificGAME_SERVICES_VERSE8_EVIDENCE_VERIFIERbinding can override it.Release
A Sampo changeset covers
@mpgd/adapter-verse8,@mpgd/catalog,@mpgd/game-services,@mpgd/target-config, and@mpgd/cli. The Verse8 adapter is already registered on npm and Trusted Publishing is configured for the repository release workflow.Validation
pnpm checkpnpm testplus the corrected remaining prepared smoke sequencepnpm validate:adspnpm validate:target-configpnpm validate:effective-configpnpm validate:targetspnpm validate:publicpnpm smoke:effective-configpnpm smoke:adapter-effective-configpnpm smoke:game-configpnpm graph:preflightpnpm pack:packagespnpm build:verse8pnpm smoke:target verse8sampo release --dry-runfrom an isolatedmaincheckoutSummary by CodeRabbit
New Features
Bug Fixes
Documentation