Repository navigation
Support configured web deployment targets - #125
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe PR adds additive custom target configuration, configurable web artifacts, runtime matrix injection, target-specific catalog mappings, deployment-aware game services, and validation coverage. ChangesConfigured Web Targets
Estimated code review effort: 5 (Critical) | ~120 minutes 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 review |
✅ Action performedReview finished.
|
![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 (1)
tools/target/target-config-matrix.ts (1)
10-19: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueShare
MPGD_TARGET_CONFIG_EXTENSIONS_FILEfrom one source of truth.tools/target/target-config-matrix.tsdeclarestargetConfigExtensionsFileEnv, butpackages/cli/src/index.tsandtools/target/build-target.tsstill repeat the literal. Replace those literals with an importable constant, or centralize the env-var name behind a public API that the CLI can use.🤖 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/target/target-config-matrix.ts` around lines 10 - 19, Centralize the MPGD_TARGET_CONFIG_EXTENSIONS_FILE environment-variable name by exposing and reusing targetConfigExtensionsFileEnv from target-config-matrix.ts. Update packages/cli/src/index.ts and the corresponding usage in tools/target/build-target.ts to import and use that shared constant instead of repeating the literal; no direct change is required at the declaration site beyond making it publicly importable if needed.
🤖 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 `@tools/target/build-target.ts`:
- Around line 159-165: The build path around staticDir handling must enforce
installable:false in the final artifact by preventing or removing any
manifest.webmanifest and manual manifest link, including files pre-existing in
output and introduced by copyDirectoryContents. Update the relevant staging/copy
flow to ensure these artifacts fail the build or are removed, and add a
build-path test covering both a pre-existing manifest and a staticDir overlay
when installable is false.
In `@tools/target/platform-targets.ts`:
- Around line 98-99: Update validatePlatformTargetsFile to reject blank
staticDir values before resolving the path, rather than relying on
assertOptionalString, while continuing to allow the property to be omitted.
Apply the same validation in the corresponding target-validation branch
referenced by the additional location, and preserve existing behavior for
non-empty paths.
In `@tools/target/validate-platform-targets.ts`:
- Around line 30-36: Update the web-target validation around target.staticDir to
use statSync(resolvePath(target.staticDir)).isDirectory() alongside the existing
existence check. Reject regular files with the current validation error before
copyDirectoryContents runs, while preserving valid directory handling.
---
Nitpick comments:
In `@tools/target/target-config-matrix.ts`:
- Around line 10-19: Centralize the MPGD_TARGET_CONFIG_EXTENSIONS_FILE
environment-variable name by exposing and reusing targetConfigExtensionsFileEnv
from target-config-matrix.ts. Update packages/cli/src/index.ts and the
corresponding usage in tools/target/build-target.ts to import and use that
shared constant instead of repeating the literal; no direct change is required
at the declaration site beyond making it publicly importable if needed.
🪄 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 Plus
Run ID: 2d16c5ae-9c8d-4fc4-b4e2-89a29ee08f22
📒 Files selected for processing (23)
.sampo/changesets/configured-web-targets.mddocs/GAME_DEVELOPMENT.mdpackage.jsonpackages/catalog/src/index.test.tspackages/catalog/src/index.tspackages/cli/src/build-targets.tspackages/cli/src/index.tspackages/target-config/src/effective.tspackages/target-config/test/target-config-smoke.tstools/icons/icons.test.tstools/icons/staging.tstools/smoke/cli-configured-web-targets.tstools/smoke/target-config-extensions.tstools/target/build-target.tstools/target/effective-config.tstools/target/generate-release-manifest.tstools/target/platform-targets.tstools/target/schemas.tstools/target/target-config-matrix.tstools/target/validate-platform-targets.tstools/validate-ad-placements.tstools/validate-product-catalog.tstools/validate-target-config.ts
💤 Files with no reviewable changes (1)
- packages/target-config/src/effective.ts
![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: 8711f8b852
ℹ️ 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 review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 136e7dd21c
ℹ️ 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 review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/target/platform-targets.ts (1)
72-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused helper.
appTargetForPlatformTargethas no caller, so keep this change scoped to the existing validation paths or wire the app-directory logic into the callers before exporting the helper.🤖 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/target/platform-targets.ts` around lines 72 - 77, Remove the unused appTargetForPlatformTarget helper and its export, since it has no callers. Keep the change limited to removing this dead code; do not add new call sites or alter the existing validation paths.
🤖 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 `@examples/phaser-starter/vite.shared.ts`:
- Around line 81-93: The readRuntimeTargetConfigMatrix functions accept
unvalidated JSON and must validate the parsed value with
assertTargetConfigMatrix before returning it. Apply this change in
examples/phaser-starter/vite.shared.ts and
packages/cli/templates/phaser-game/vite.shared.ts at the indicated ranges,
preserving the existing empty-input handling and parse-error wrapping.
---
Nitpick comments:
In `@tools/target/platform-targets.ts`:
- Around line 72-77: Remove the unused appTargetForPlatformTarget helper and its
export, since it has no callers. Keep the change limited to removing this dead
code; do not add new call sites or alter the existing validation paths.
🪄 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 Plus
Run ID: 61322f1e-e9fd-4893-b0c3-0055c74f7c7d
📒 Files selected for processing (23)
.sampo/changesets/configured-web-targets.mdexamples/phaser-starter/src/env.d.tsexamples/phaser-starter/src/platform/installStarterPlatform.tsexamples/phaser-starter/vite.shared.tspackage.jsonpackages/cli/src/build-targets.tspackages/cli/src/index.tspackages/cli/src/target-config-env.tspackages/cli/src/target-name.tspackages/cli/templates/phaser-game/src/env.d.tspackages/cli/templates/phaser-game/src/platform/installPlatform.tspackages/cli/templates/phaser-game/vite.shared.tstools/icons/icons.test.tstools/icons/staging.tstools/smoke/cli-configured-web-targets.tstools/smoke/target-config-extensions.tstools/target/build-target.tstools/target/effective-config.tstools/target/platform-targets.tstools/target/target-config-matrix.tstools/target/validate-platform-targets.tstools/target/web-artifact.test.tstools/target/web-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- tools/target/validate-platform-targets.ts
- tools/target/target-config-matrix.ts
- tools/target/effective-config.ts
- tools/icons/staging.ts
- tools/icons/icons.test.ts
- .sampo/changesets/configured-web-targets.md
- tools/smoke/cli-configured-web-targets.ts
- package.json
- packages/cli/src/build-targets.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0da769e0bc
ℹ️ 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".
|
Verified the appTargetForPlatformTarget helper is used by tools/target/build-target.ts and tools/smoke/target-config-extensions.ts, so it is intentionally retained. |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f229bd3011
ℹ️ 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 review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5b499cfb3
ℹ️ 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 review |
|
@codex review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3045f4fa3
ℹ️ 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 review |
|
@codex review |
|
@codex review Please re-review the current head |
|
@coderabbitai review Please re-review the current head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/game-services/src/types.ts (1)
317-346: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBlock prototype keys for catalog target lookups.
resolveProductPlatformIdindexesproduct.platformProductIds[target], so values likeconstructorcan match inherited object properties instead of configured product IDs. Add those keys toreservedDeploymentTargetNamesor make the target lookup use own-property checks / aMap.🤖 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/types.ts` around lines 317 - 346, Update deployment target validation and lookup behavior around assertOptionalDeploymentTarget and resolveProductPlatformId so prototype keys such as constructor and prototype cannot resolve as configured catalog targets. Prefer an own-property check for product.platformProductIds or reserve every relevant prototype key in reservedDeploymentTargetNames, while preserving valid configured target lookups.
🤖 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 `@tools/target/web-artifact.ts`:
- Around line 462-494: Update the implicit-head-close logic in scanHtmlHead so
disallowed start tags also close an explicitly opened head: apply the
implicitHeadElementNames check whenever implicitHeadClosed is false, regardless
of inHead, and set inHead = false when closing it. Preserve existing handling
for allowed head elements and already-closed heads.
---
Outside diff comments:
In `@packages/game-services/src/types.ts`:
- Around line 317-346: Update deployment target validation and lookup behavior
around assertOptionalDeploymentTarget and resolveProductPlatformId so prototype
keys such as constructor and prototype cannot resolve as configured catalog
targets. Prefer an own-property check for product.platformProductIds or reserve
every relevant prototype key in reservedDeploymentTargetNames, while preserving
valid configured target lookups.
🪄 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 Plus
Run ID: c9e07e9e-95d2-4a60-a832-97630d6c53ec
📒 Files selected for processing (11)
.sampo/changesets/configured-web-targets.mdpackages/game-services/src/client.test.tspackages/game-services/src/client.tspackages/game-services/src/server.test.tspackages/game-services/src/server.tspackages/game-services/src/types.tstools/icons/icons.test.tstools/icons/staging.tstools/target/build-target.tstools/target/web-artifact.test.tstools/target/web-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/game-services/src/client.ts
- packages/game-services/src/client.test.ts
- packages/game-services/src/server.test.ts
- tools/icons/icons.test.ts
- tools/icons/staging.ts
- packages/game-services/src/server.ts
- .sampo/changesets/configured-web-targets.md
- tools/target/web-artifact.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cb30ffc92
ℹ️ 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 Please re-review the current head |
|
@coderabbitai review Please re-review the current head |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cb7358874
ℹ️ 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 Please review the current head ( |
|
@coderabbitai review Please review the current head ( |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Current head and changed files ==='
git rev-parse --short HEAD
git diff --name-status HEAD^ HEAD
git diff --stat HEAD^ HEAD
echo '=== Relevant files ==='
fd -t f . \
| rg '(^|/)(server(\.test)?|web-artifact(\.test)?|agent8(\.test)?|.*verse8.*|.*workflow.*)\.(ts|ya?ml)$' \
|| true
echo '=== Source outlines ==='
for f in \
packages/game-services/src/server.ts \
packages/game-services/src/server.test.ts \
tools/target/web-artifact.ts \
tools/target/web-artifact.test.ts \
adapters/verse8/src/agent8.ts \
adapters/verse8/src/agent8.test.ts
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all || true
fi
done
echo '=== Focused diff ==='
git diff --find-renames --find-copies HEAD^ HEAD -- \
packages/game-services/src/server.ts \
packages/game-services/src/server.test.ts \
tools/target/web-artifact.ts \
tools/target/web-artifact.test.ts \
adapters/verse8 \
.github \
| sed -n '1,2600p'Length of output: 43920 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Server binding and target resolution ==='
sed -n '315,405p' packages/game-services/src/server.ts
sed -n '550,640p' packages/game-services/src/server.ts
sed -n '640,750p' packages/game-services/src/server.ts
sed -n '940,1045p' packages/game-services/src/server.ts
sed -n '1,150p' packages/game-services/src/types.ts
echo '=== Backend construction call sites ==='
rg -n -C 4 'createGameServicesBackend\s*\(|deploymentTargetBindings|deploymentTarget:' \
--glob '*.ts' \
packages adapters examples tools \
| sed -n '1,1800p'
echo '=== Static-directory and protected-output logic ==='
sed -n '140,230p' tools/target/web-artifact.ts
sed -n '580,820p' tools/target/web-artifact.ts
sed -n '1,130p' tools/target/validate-platform-targets.ts
sed -n '1,180p' tools/target/platform-targets.ts
sed -n '360,430p' tools/target/web-artifact.test.ts
echo '=== HTML head scanner ==='
sed -n '250,430p' tools/target/web-artifact.ts
sed -n '430,590p' tools/target/web-artifact.ts
echo '=== Verse8 CI job context ==='
sed -n '55,110p' .github/workflows/ci.yml
rg -n -C 4 'VITE_MPGD_GAME_SERVICES_URL|smoke:target verse8|verse8-agent8' \
package.json .github tools adapters packages examples \
| sed -n '1,1000p'Length of output: 107815 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Build preflight and static-overlay paths ==='
rg -n -C 5 \
'validatePlatformTargetsFile\s*\(|assertDisjointWebTargetOutputs\s*\(|assertWebStaticDirectory\s*\(|copyWebStaticDirectoryContents\s*\(' \
tools packages \
--glob '*.ts' \
| sed -n '1,1800p'
echo '=== Deployment-target validation ==='
rg -n -C 5 \
'function assertGameServicesDeploymentTarget|assertGameServicesDeploymentTarget\s*=|assertOptionalGameServicesDeploymentTarget|assertDeploymentTargetName' \
packages/game-services/src packages/cli/src \
--glob '*.ts' \
| sed -n '1,900p'
echo '=== Focused server fixtures and assertions ==='
sed -n '15,115p' packages/game-services/src/server.test.ts
sed -n '240,415p' packages/game-services/src/server.test.ts
sed -n '315,425p' tools/target/build-target.ts
sed -n '1,210p' tools/target/web-artifact.test.ts
echo '=== Verse8 target configuration ==='
rg -n -C 5 '"verse8"|verse8' \
examples/phaser-starter/mpgd.targets.json \
packages/target-config/targets.json \
| sed -n '1,600p'
echo '=== Full current commit patch ==='
git show --format=fuller --find-renames --find-copies --stat --patch HEAD \
| sed -n '1,3600p'Length of output: 76517
I found no actionable issues in the requested areas.
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/cli/src/production-target-readiness.ts`:
- Around line 64-70: Update the ownerPathKey === undefined branch in the
production-target readiness validation to reject configurations where
requiresVerse8RewardAuthority is true and authoritativeGameServices is
explicitly false before calling assertAuthoritativeGameServicesUrl or returning.
Preserve the existing URL validation for other configurations and ensure the
explicit non-authoritative flag cannot bypass the rewarded-ad requirement.
- Around line 17-27: Update assertProductionTargetReadiness to reject production
reads for Verse8 targets when targetPolicy is absent, ensuring the validation
cannot bypass readTargets and assertAuthoritativeGameServicesUrl through an
undefined ownerPathKey. Apply the requirement to all non-built-in
ait/android/ios owner-path targets while preserving existing behavior for
built-in 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 Plus
Run ID: b1e6a041-5f97-4a1b-a2f1-4388f2fe0090
📒 Files selected for processing (11)
.github/workflows/ci.ymlpackages/catalog/src/index.test.tspackages/catalog/src/index.tspackages/cli/src/production-target-readiness.tspackages/game-services/src/server.test.tspackages/game-services/src/server.tspackages/game-services/src/types.tstools/smoke/production-target-readiness.tstools/target/build-target.tstools/target/web-artifact.test.tstools/target/web-artifact.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/catalog/src/index.test.ts
- packages/game-services/src/types.ts
- tools/target/web-artifact.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe95d9a5d0
ℹ️ 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 Please re-review the current head ( |
|
@coderabbitai review Please re-review the current head ( |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Keep it up! 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 review Please retry the final review of the current head ( |
|
✅ Action performedReview finished.
|
Summary
kind: "web"target names in CLI build and matrix commandsWhy
Independent browser deployments can require distinct runtime policies and release artifacts while still sharing the standard web builder. Keeping those names and files game-owned avoids adding deployment-specific concepts to the public target matrix.
Validation
mise exec -- pnpm checkmise exec -- pnpm testSAMPO_RELEASE_BRANCH=main mise exec -- pnpm sampo:release:dry-runSummary by CodeRabbit
New Features
Bug Fixes
Documentation