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 (8)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds game-owned web-target configuration, shared CLI target normalization, custom catalog mappings, configurable web artifacts, runtime matrix injection, and smoke coverage for extension loading and configured web targets. ChangesConfigured Web Targets
Estimated code review effort: 4 (Complex) | ~60 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 |
|
@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: 5b3b662b43
ℹ️ 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
🤖 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 102-110: Update the web-target preflight around webTargetPaths and
assertWebStaticDirectory to canonicalize target.output and ${gameApp}/dist, then
reject the configuration when they resolve to the same directory, including
normalized relative paths and symlink-equivalent paths. Keep the existing
staticDir validations unchanged and report the conflicting output path clearly.
🪄 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: eac0b32b-a5a8-4670-a5b6-d6ff09ee23b6
📒 Files selected for processing (13)
.sampo/changesets/configured-web-targets.mdpackages/cli/src/target-name.tstools/icons/icons.test.tstools/icons/profiles.tstools/icons/staging.tstools/smoke/cli-configured-web-targets.tstools/smoke/target-config-extensions.tstools/target/build-target.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 (7)
- tools/target/validate-platform-targets.ts
- packages/cli/src/target-name.ts
- tools/smoke/target-config-extensions.ts
- tools/smoke/cli-configured-web-targets.ts
- tools/icons/profiles.ts
- tools/icons/icons.test.ts
- tools/target/platform-targets.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". |
|
@codex review |
|
@coderabbitai 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: bc0a5c4be0
ℹ️ 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: 14586dfdbc
ℹ️ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Breezy! 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". |
1 participant
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